openSUSE / open-build-service

Build and distribute Linux packages from sources in an automatic, consistent and reproducible way #obs
https://openbuildservice.org
GNU General Public License v2.0
925 stars 436 forks source link

Replace `require_admin` controller filters with Pundit policies #10084

Open dmarcoux opened 4 years ago

dmarcoux commented 4 years ago

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.

PapePathe commented 4 years ago

Hi i'm interested for working on this.

dmarcoux commented 4 years ago

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

PapePathe commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

Sounds like i need additional setup ?

DavidKang commented 4 years ago

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 commented 4 years ago

Thanks @DavidKang

PapePathe commented 4 years ago

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 commented 4 years ago

@PapePathe, are you using Linux or MacOS?

PapePathe commented 4 years ago

MacOS

DavidKang commented 4 years ago

@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 :slightly_smiling_face:

PapePathe commented 4 years ago

Thanks.

DavidKang commented 4 years ago

@PapePathe, I found it :smiley: Setup an OBS Development Environment on macOS

PapePathe commented 4 years ago

Thaaaanks.

PapePathe commented 4 years ago

It worked @DavidKang. Thanks again.

Now i can start digging in the code.

DavidKang commented 4 years ago

You are welcome. Have fun 😁

PapePathe commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

I will thanks.

PapePathe commented 4 years ago

Hi i have an issue running the test suite.

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

dmarcoux commented 4 years ago

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 commented 4 years ago

Thanks @dmarcoux