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

Add REQUIRE_EMAIL_CONFIRM setting #7

Merged
merged 2 commits into from
Jan 18, 2017
Merged

Conversation

windy1
Copy link
Contributor

@windy1 windy1 commented Jan 16, 2017

Signed-off-by: Walker Crouse walkercrouse@hotmail.com

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling c6b70f1 on windy1:conf into c4ac9b2 on lukegb:master.

Copy link
Collaborator

@lukegb lukegb left a comment

Choose a reason for hiding this comment

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

Can you also add a test to

https://github.com/lukegb/SpongeAuth/blob/0295e712ee0a650b86242d9d9a681912c9c72580/spongeauth/accounts/tests/test_middleware_enforce_verified_emails.py

in TestMustVerify to make sure that it returns False if REQUIRE_EMAIL_CONFIRM is False, even if the other conditions are met?


@staticmethod
def may_pass(url):
try:
return getattr(
resolve(url).func, 'allow_without_verified_email', False)
resolve(url).func, 'allow_without_verified_email', False) or not settings.REQUIRE_EMAIL_CONFIRM
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think you need this here as well.

@@ -11,6 +11,8 @@

SECRET_KEY = os.environ['SECRET_KEY']

REQUIRE_EMAIL_CONFIRM = True
Copy link
Collaborator

Choose a reason for hiding this comment

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

If this is true in base it's probably not worthwhile setting it to True here.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling c97dc65 on windy1:conf into 92c6482 on lukegb:master.

Signed-off-by: Walker Crouse <walkercrouse@hotmail.com>
@windy1
Copy link
Contributor Author

windy1 commented Jan 17, 2017

@lukegb Updated

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling f877717 on windy1:conf into 92c6482 on lukegb:master.

@windy1 windy1 force-pushed the conf branch 2 times, most recently from f877717 to 6c117b1 Compare January 17, 2017 18:58
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 6c117b1 on windy1:conf into 92c6482 on lukegb:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling e5c15d1 on windy1:conf into 92c6482 on lukegb:master.

@lukegb
Copy link
Collaborator

lukegb commented Jan 18, 2017

Thanks!

@lukegb lukegb merged commit 8dd147c into SpongePowered:master Jan 18, 2017
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.

None yet

3 participants