SSO - #431
Open
jicampos wants to merge 3 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces initial Single Sign-On (SSO) support using an OpenID Connect (OIDC) authorization-code flow in GatewaySupervisor, along with related WebUsers utilities updates and a small robustness improvement to the tools/ots launcher.
Changes:
- Add OIDC login and callback endpoints in
GatewaySupervisor(including config parsing and token exchange via libcurl). - Extend
WebUsersto support creating/activating sessions based on externally authenticated email identities. - Add a relaunch-attempt limit to the gateway-context launch loop in
tools/ots.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/ots | Adds a bounded retry loop when gateway context launch can’t be verified. |
| otsdaq/WebUsersUtilities/WebUsers.h | Extends the WebUsers API to support email-based SSO session activation and configurable initial permissions on account creation. |
| otsdaq/WebUsersUtilities/WebUsers.cc | Implements email-based session activation and initial-permission support for new accounts. |
| otsdaq/GatewaySupervisor/GatewaySupervisor.h | Adds XGI handlers for OIDC login and callback endpoints. |
| otsdaq/GatewaySupervisor/GatewaySupervisor.cc | Implements OIDC discovery, authorization redirect, code exchange, and session activation glue. |
| otsdaq/GatewaySupervisor/CMakeLists.txt | Links GatewaySupervisor against libcurl for OIDC HTTP requests. |
Suppressed comments (2)
otsdaq/GatewaySupervisor/GatewaySupervisor.cc:10889
- The callback trusts the decoded
id_tokenpayload without validating the JWT signature or required OIDC claims (at leastiss,aud, andexp). Without token validation, a forged/replayed token could be accepted as a login identity.
// decode the ID token claims and extract the user's identity
std::string claims = oidcJwtPayload(idToken);
if(claims == "")
{
oidcErrorPage(out, "Could not decode ID token");
otsdaq/WebUsersUtilities/WebUsers.cc:1661
- Auto-created SSO accounts are currently granted
PERMISSION_LEVEL_EXPERT(100), even though the comment above says they should be created with novice permissions. This grants elevated privileges to any authenticated SSO user.
createNewAccount(newUsername,
newDisplayName,
email,
WebUsers::PERMISSION_LEVEL_EXPERT /*initial permission (100)*/);
Comment on lines
1132
to
+1136
| // first user is admin always! | ||
| std::map<std::string /*groupName*/, WebUsers::permissionLevel_t> initPermissions = { | ||
| {WebUsers::DEFAULT_USER_GROUP, | ||
| (Users_.size() ? WebUsers::PERMISSION_LEVEL_NOVICE | ||
| : WebUsers::PERMISSION_LEVEL_ADMIN)}}; | ||
| (Users_.size() ? initialPermission | ||
| : WebUsers::permissionLevel_t(WebUsers::PERMISSION_LEVEL_ADMIN))}}; |
Comment on lines
+1625
to
+1627
| for(const char& c : email.substr(0, email.find('@'))) | ||
| if(isalnum(c) || c == '_' || c == '-' || c == '.') | ||
| baseUsername += c; |
Comment on lines
+10652
to
+10659
| std::string oidcGenRandomHex(unsigned int length) | ||
| { | ||
| static const char hex[] = "0123456789abcdef"; | ||
| std::string out; | ||
| for(unsigned int i = 0; i < length; ++i) | ||
| out += hex[rand() % 16]; | ||
| return out; | ||
| } // end oidcGenRandomHex() |
Comment on lines
+10679
to
+10684
| if((unsigned char)c < 0x20) | ||
| { | ||
| char buf[8]; | ||
| snprintf(buf, sizeof(buf), "\\u%04x", c); | ||
| out += buf; | ||
| } |
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
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.
Related to PR art-daq/otsdaq-utilities#361
For issue art-daq/otsdaq-utilities#351