Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace require_admin controller filters with Pundit policies #10084

Open
dmarcoux opened this issue Aug 27, 2020 · 22 comments
Open

Replace require_admin controller filters with Pundit policies #10084

dmarcoux opened this issue Aug 27, 2020 · 22 comments
Labels
Frontend Things related to the OBS RoR app Refactoring 🏭

Comments

@dmarcoux
Copy link
Contributor

Some controllers rely solely on Pundit and others on custom legacy code like require_admin to authorize various actions throughout the application.

We want to rely completely on Pundit to use a widely adopted authorization system instead of our own custom solution. Replacing require_admin controller filters with Pundit policies brings us a step closer to this goal.

To make this more manageable, this issue should be tackled controller by controller.

@dmarcoux dmarcoux added Frontend Things related to the OBS RoR app Refactoring 🏭 good first issue Easy task, perfect for a first contribution labels Aug 27, 2020
@PapePathe
Copy link

Hi i'm interested for working on this.

@dmarcoux
Copy link
Contributor Author

dmarcoux commented Sep 10, 2020

Hey @PapePathe, thank you for offering to help :). Let us know if something isn't clear.

@PapePathe
Copy link

Cool. I cloned the repo yesterday.
I'll try to finish the setup by the week end and then start digging in the code.

@PapePathe
Copy link

Hi @dmarcoux i have a database connection issue.

I followed the instructions in CONTRIBUTING.md but i have this error when i run "docker-compose up"

frontend_1 | 16:19:02 clock.1 | I, [2020-09-10T16:19:02.005297 #10] INFO -- : Triggering 'send notifications' frontend_1 | 16:19:08 clock.1 | I, [2020-09-10T16:19:08.001380 #10] INFO -- : Triggering 'fetch notifications' frontend_1 | 16:19:08 clock.1 | E, [2020-09-10T16:19:08.014125 #10] ERROR -- : Failed to open TCP connection to backend:5352 (Connection refused - connect(2) for "backend" port 5352) (Errno::ECONNREFUSED)

@PapePathe
Copy link

Sounds like i need additional setup ?

@DavidKang
Copy link
Contributor

DavidKang commented Sep 10, 2020

uhmm @PapePathe , if it's the database, please check if in config/database.yml you have the host: db. If it's not the case, you need to add:

host: db

in the development and test environment.

In other hand, that sounds more an issue of the backend. Just to be sure, have you run the next commands?

git submodule init
git submodule update

@PapePathe
Copy link

Thanks @DavidKang

@PapePathe
Copy link

I have the log below in the docker-compose output

backend_1 | Can't locate Build/Rpm.pm in @INC (you may need to install the Build::Rpm module) (@INC contains: /obs/src/backend /obs/src/backend/build /usr/lib/perl5/site_perl/5.26.1/x86_64-linux-thread-multi /usr/lib/perl5/site_perl/5.26.1 /usr/lib/perl5/vendor_perl/5.26.1/x86_64-linux-thread-multi /usr/lib/perl5/vendor_perl/5.26.1 /usr/lib/perl5/5.26.1/x86_64-linux-thread-multi /usr/lib/perl5/5.26.1 /usr/lib/perl5/site_perl) at /obs/src/backend/build/Build.pm line 25. backend_1 | BEGIN failed--compilation aborted at /obs/src/backend/build/Build.pm line 25.

@DavidKang
Copy link
Contributor

@PapePathe, are you using Linux or MacOS?

@PapePathe
Copy link

MacOS

@DavidKang
Copy link
Contributor

@PapePathe, with MacOs you need to check if your file system is case sensitive. In the second paragraph of the Contribution Guide we talk about that.

I hope that helps a bit 🙂

@PapePathe
Copy link

Thanks.

@DavidKang
Copy link
Contributor

@PapePathe
Copy link

Thaaaanks.

@PapePathe
Copy link

It worked @DavidKang. Thanks again.

Now i can start digging in the code.

@DavidKang
Copy link
Contributor

You are welcome. Have fun 😁

@PapePathe
Copy link

PapePathe commented Sep 10, 2020

Any advices about where to start on this task.

Ex:

  1. Create a policy for model A
  2. Use pundit authorize method in controller B

@DavidKang
Copy link
Contributor

Well, before start coding. I would check which policies exists, and how is used in this project. Then you can decide if you need to a new policies or just apply it in the controller.

Let us know if something isn't clear or if you need help 😃.

You can also create a PR with your solution and receive feedback 😁

@PapePathe
Copy link

I will thanks.

@PapePathe
Copy link

Hi i have an issue running the test suite.

It is reporting a missing record from the db --> Configuration.first

@dmarcoux
Copy link
Contributor Author

Seeds are missing in the test database. You can add them to the test database by running this command inside the frontend Docker container:

RAILS_ENV=test bundle exec rake db:seed

To run commands in a Docker container, please refer to https://github.com/openSUSE/open-build-service/wiki/Development-Environment-Tips-&-Tricks#run-commands-in-service

@PapePathe
Copy link

Thanks @dmarcoux

@dmarcoux dmarcoux removed the good first issue Easy task, perfect for a first contribution label Nov 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Frontend Things related to the OBS RoR app Refactoring 🏭
Projects
None yet
Development

No branches or pull requests

3 participants