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

Authorization #17

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Authorization #17

wants to merge 16 commits into from

Conversation

nikolai-b
Copy link
Contributor

Turned out to be a bit more work than we expected but a long train ride later and it looks ok. I'll have a play around in dev soon but the tests pass.
In the users_controller I removed the 'owner' role and replaced with 'manager'. Not sure what the roles table looks like in prod so maybe we should add some methods for 'authorize_owner'.

Cheers!

@nikolai-b
Copy link
Contributor Author

hi, are you still interested in this? I looked at the code I wrote and it was a bit ropey so I tidied it up a bit.

@asalant
Copy link
Owner

asalant commented Jan 16, 2015

Definitely still interested though probably going to be a bit before I can
get around to it. Thanks for checking in!

On Fri, Jan 2, 2015 at 12:10 PM, Nikolai notifications@github.com wrote:

hi, are you still interested in this? I looked at the code I wrote and it
was a bit ropey so I tidied it up a bit.


Reply to this email directly or view it on GitHub
#17 (comment).

permit "admin or (manager of :organization)", :only => [ :show, :edit, :update ]

before_filter :authorize_admin_or_manager, :only => [ :show, :edit, :update ]
before_filter :authorize_admin, :only => [ :destory ]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:destroy?

@asalant
Copy link
Owner

asalant commented Dec 4, 2019

Hi @nikolai-b,

I'm finally (years later!!) giving Freehub some love. This PR has been sitting for a while and I'm having trouble remembering why we were making these changes. Was it to remove the dependency on the authorization plugin or were there new features we wanted to support?

Thanks!

@nikolai-b
Copy link
Contributor Author

Hi @asalant

I could only just remember: #14 (comment)
I think if you want to get rails to a higher version then owning the authorization code will make it easier. If you've no plans to bump rails then this doesn't help much 😄

@asalant
Copy link
Owner

asalant commented Dec 5, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants