OpenID Connect Login - #3331
Closed
netpro2k wants to merge 5 commits into
Closed
Conversation
brianpeiris
reviewed
Dec 4, 2020
brianpeiris
left a comment
brianpeiris
left a comment
Contributor
There was a problem hiding this comment.
Just minor comments. Not sure how incomplete this draft is, so I'm happy to do another review later.
| return getDescriptors(categorySchema).filter( | ||
| ([path, descriptor]) => | ||
| (qs.get("show_internal_configs") !== null || descriptor.internal !== "true") && | ||
| (qs.get("show_oidc_configs") !== null || path[1] !== "oidc") |
Contributor
There was a problem hiding this comment.
Why was it necessary to add another query param for this? Isn't internal_configs sufficient?
| "sign-in.admin-no-permission": "You don't have access to admin tools. Sign into another account or ask an administrator to grant you permission.", | ||
| "sign-in.hub": "An account is required to join rooms.\n\nEnter your email to create your account or sign in.", | ||
| "sign-in.auth-started": "Email sent to {email}!\n\nTo continue, click on the link in the email using your phone, tablet, or PC.\n\nNo email? You may not be able to create an account.", | ||
| "sign-in.oidc-auth-started": "Waiting for signin...", |
Contributor
There was a problem hiding this comment.
Reminder to use the sync_locales.js script to populate the other languages.
| channel.on("auth_credentials", async ({ credentials: token, payload: payload }) => { | ||
| await this.handleAuthCredentials({ email: payload.email }, token); | ||
| resolve(); | ||
| }); |
Contributor
There was a problem hiding this comment.
Probably best to keep the previous order and register the handler before pushing the message.
|
|
||
| const authComplete = new Promise(resolve => | ||
| channel.on("auth_credentials", async ({ user_info, credentials: token }) => { | ||
| console.log("got credentials", user_info, token); |
| async handleAuthCredentials(email, token, hubChannel) { | ||
| this.store.update({ credentials: { email, token } }); | ||
| async handleAuthCredentials(userInfo, token, hubChannel) { | ||
| console.log("handleAuthCredentials", userInfo, token, hubChannel); |
Contributor
|
We can revisit this prior to the May Hubs Cloud release and decide if we want to make any additional features part of the initial alpha/experimental release |
Contributor
Author
|
Closing in favor of #5757 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds support for signing in with OpenID Connect when the Hubs instance is configured to do so. most of the changes are on the backend in Hubs-Foundation/reticulum#426
At first shipping this will not yet be a "supported" feature as we still need to figure out edge cases, mostly around changing auth providers (you will end up with orphaned accounts, and can also easily lock yourself out of the admin interface).
The admin panel options are all marked as "internal" and so will be hidden unless the
show_internal_configsquery param is added to the admin panel. Additionally the server side configs are hidden behind another query paramshow_oidc_configs, so both should be set when configuring this.┆Issue is synchronized with this Jira Task