Skip to content

Conversation

trbs
Copy link
Contributor

@trbs trbs commented May 18, 2015

Add an addition setting called APPLICATION_REGISTRATION_PERMISSIONS
which controls which permissions must be set on a user to allow the
creation of applications.

By default (in this PR) this is the 'add_application' permission which
regretfully is not backwards compatible but it does seem to make the
most sense from a security perspective as a default.

Reverting to the old behaviour is easily done by using None for the
settings key.

…ation

Add an addition setting called APPLICATION_REGISTRATION_PERMISSIONS
which controls which permissions must be set on a user to allow the
creation of applications.

By default (in this PR) this is the 'add_application' permission which
regretfully is not backwards compatible but it does seem to make the
most sense from a security perspective as a default.

Reverting to the old behaviour is easily done by using None for the
settings key.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 95.91% when pulling 3ca899f on trbs:application_permissions into 75cd656 on evonove:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 95.91% when pulling 3ca899f on trbs:application_permissions into 75cd656 on evonove:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 95.91% when pulling 3ca899f on trbs:application_permissions into 75cd656 on evonove:master.

@synasius
Copy link
Contributor

HI @trbs,
great work!

I'd prefer to leave the default behavior unchanged, so the app is backwards compatible.

Can you add some docs for the settings APPLICATION_REGISTRATION_PERMISSIONS with an example and some specifc tests?

Thanks a lot for your effort

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 95.95% when pulling ffed538 on trbs:application_permissions into 75cd656 on evonove:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 95.95% when pulling ffed538 on trbs:application_permissions into 75cd656 on evonove:master.

@trbs
Copy link
Contributor Author

trbs commented May 18, 2015

@synasius the default is now backwards compatible and I put the old settings as the example in the documentation. Two extra tests now test creating an application with and without the correct permission.

Copy link
Contributor

Choose a reason for hiding this comment

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

s/none/non/

@masci masci added this to the 0.9.0 milestone May 23, 2015
@pySilver
Copy link

pySilver commented Oct 8, 2015

Isn't it better to have a configurable callback that tests if user has rights to create apps? Originally it won't be defined and would do nothing but in real life experience one can supply own callback which will receive lets say User instance and will be expected to return boolean. So, implementation would be up to the developer.

@palazzem palazzem modified the milestones: 0.9.0, 0.10.0 Oct 15, 2015
@synasius
Copy link
Contributor

@pySilver

Isn't it better to have a configurable callback that tests if user has rights to create apps?

Definitely! That way we're not bound to how django handles permissions. I'm closing this PR.
@trbs thanks for the great work but it's not what we're looking for.

@synasius synasius closed this Nov 25, 2015
@trbs trbs deleted the application_permissions branch November 25, 2015 16:51
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.

6 participants