- Notifications
You must be signed in to change notification settings - Fork927
fix: assign new oauth users to default org#12145
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Emyrk commentedFeb 14, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
This stack of pull requests is managed by Graphite.Learn more about stacking. |
60775a6 to04c6149Comparef999de5 to5322f4eCompare| organization, _ := tx.GetDefaultOrganization(dbauthz.AsSystemRestricted(ctx)) | ||
| // Add the user to the default organization. | ||
| // Once multi-organization we should check some configuration to see | ||
| // if we should add the user to a different organization. | ||
| organizationID = organization.ID |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is annoyingly allowed to beuuid.Nil in unit tests. I am not going to change the behavior here, but we should really fix that so we don't have an edge case living for the sake of tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Fine not to fix here, but can we add a comment for the ignored err above (tx.GetDefaultOrganization) about this? Comment is just a band-aid though, we definitely want to nip this in the bud before too long.
04c6149 to7059828Compare5322f4e toddb828cCompareaeb29f2 to8f61a71Compareddb828c tofc2fce2CompareThis is not a final solution, as we eventually want to be ableto map to different orgs. This makes it so multi-org does not break oauth/oidc.
fc2fce2 to872f4a2Compare| organization, _ := tx.GetDefaultOrganization(dbauthz.AsSystemRestricted(ctx)) | ||
| // Add the user to the default organization. | ||
| // Once multi-organization we should check some configuration to see | ||
| // if we should add the user to a different organization. | ||
| organizationID = organization.ID |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Fine not to fix here, but can we add a comment for the ignored err above (tx.GetDefaultOrganization) about this? Comment is just a band-aid though, we definitely want to nip this in the bud before too long.
Uh oh!
There was an error while loading.Please reload this page.
Begins work on#11922
This is not a final solution, as we eventually want to be able to map to different orgs.
This PR just makes it so multi-org does not break oauth/oidc.