Skip to content
This repository has been archived by the owner on Feb 22, 2024. It is now read-only.

Remove active duplicate with database default #228

Closed
wants to merge 1 commit into from
Closed

Remove active duplicate with database default #228

wants to merge 1 commit into from

Conversation

Diaoul
Copy link
Contributor

@Diaoul Diaoul commented Mar 12, 2014

This duplicates with the default value in the database and doesn't allow creation of inactive users without a lot of code.

Use case: inactive user self-registration with manual activation by an administrator

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 54161f0 on Diaoul:develop into 1d3a75d on mattupstate:develop.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 54161f0 on Diaoul:develop into 1d3a75d on mattupstate:develop.

@mattupstate
Copy link
Collaborator

Not sure I understand the issue here. There is no problem with passing active=False to the datastore.create_user. This is tested here:

https://github.com/mattupstate/flask-security/blob/develop/tests/functional_tests.py#L60-L62

@Diaoul
Copy link
Contributor Author

Diaoul commented Mar 13, 2014

Yes but default behavior upon registration is to set the user as active. Hence, when a user registers itself, it is active by default.
I want newly registered users to be inactive by default so that some admin do some validation and activates them afterwards.

https://github.com/mattupstate/flask-security/blob/0268a2d56834a55ad0fdbadb4533e8794b1fd0d6/flask_security/views.py#L118
The dict data is passed to the datastore.create_user as **kwargs. It does not contain the active part.

@mattupstate
Copy link
Collaborator

I understand you better now. Thanks for the explanation. However, I'm not sure this is a common enough use case at the moment.

@Diaoul
Copy link
Contributor Author

Diaoul commented Mar 13, 2014

I agree this is not a common case and as I don't want to monkey patch I do use a role instead.

However, setting active by default duplicates with the database default value. IMO, you should rely on default database value for that.

Another solution if you want it more backward compatible is to add a SECURITY_DEFAULT_ACTIVE configuration option that defaults to True to keep the behavior.
This adds complexity for something quite simple but this is fully backward compatible.

@mattupstate
Copy link
Collaborator

If you were to update your pull request and add tests to cover your feature I might consider it.

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

Successfully merging this pull request may close these issues.

None yet

3 participants