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

Fix provider state population to the server registry #263

Merged
merged 4 commits into from
Jul 31, 2018

Conversation

ChristophWurst
Copy link
Member

@ChristophWurst ChristophWurst commented Jul 30, 2018

Required for https://github.com/orgs/nextcloud/projects/4#card-10826964

This PR consists of two parts. First, a little refactoring to decouple some logic from the provider to decouple it from the necessary action on enabled/disabled state change via the server event system. Second, it adds a new state change listener that populates the state to the server's provider registry.

@rullzer please have a look :)

@ChristophWurst
Copy link
Member Author

Integration test fails due to a bug that I'm about to fix (which I've actually done this refactoring for).

@ChristophWurst ChristophWurst changed the title Refactor state change to use events Fix provider state population to the server registry Jul 31, 2018
@ChristophWurst
Copy link
Member Author

Obviously, CI fails now because the server part is missing. Let me quickly change the branch to test this temporarily …

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@ChristophWurst ChristophWurst force-pushed the refactor/state-change-events branch from 4ecc594 to 17900cf Compare July 31, 2018 07:01
ChristophWurst added a commit to nextcloud/server that referenced this pull request Jul 31, 2018
Starting with Nextcloud 14, the server knows the enabled/disabled
state of 2fa providers. While it will query that information if it's
unknown (on first use), it won't notice any changes. Thus, providers
have to propagate that information themselves.

Ref nextcloud/twofactor_totp#263
Ref nextcloud/twofactor_u2f#210

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
ChristophWurst added a commit to nextcloud/server that referenced this pull request Jul 31, 2018
Starting with Nextcloud 14, the server knows the enabled/disabled
state of 2fa providers. While it will query that information if it's
unknown (on first use), it won't notice any changes. Thus, providers
have to propagate that information themselves.

Ref nextcloud/twofactor_totp#263
Ref nextcloud/twofactor_u2f#210

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
ChristophWurst added a commit to nextcloud/server that referenced this pull request Jul 31, 2018
Starting with Nextcloud 14, the server knows the enabled/disabled
state of 2fa providers. While it will query that information if it's
unknown (on first use), it won't notice any changes. Thus, providers
have to propagate that information themselves.

Ref nextcloud/twofactor_totp#263
Ref nextcloud/twofactor_u2f#210

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

Looks good. 🚀

@ChristophWurst
Copy link
Member Author

Thanks for your review 🕺

@ChristophWurst ChristophWurst merged commit 22631d5 into master Jul 31, 2018
@ChristophWurst ChristophWurst deleted the refactor/state-change-events branch July 31, 2018 11:34
ChristophWurst added a commit to nextcloud/server that referenced this pull request Aug 1, 2018
Starting with Nextcloud 14, the server knows the enabled/disabled
state of 2fa providers. While it will query that information if it's
unknown (on first use), it won't notice any changes. Thus, providers
have to propagate that information themselves.

Ref nextcloud/twofactor_totp#263
Ref nextcloud/twofactor_u2f#210

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
weeman1337 pushed a commit to nextcloud/server that referenced this pull request Aug 9, 2018
Starting with Nextcloud 14, the server knows the enabled/disabled
state of 2fa providers. While it will query that information if it's
unknown (on first use), it won't notice any changes. Thus, providers
have to propagate that information themselves.

Ref nextcloud/twofactor_totp#263
Ref nextcloud/twofactor_u2f#210

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants