-
Notifications
You must be signed in to change notification settings - Fork 3
add saml support (tested with okta saml) #163
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
base: main
Are you sure you want to change the base?
Changes from 10 commits
c18f23f
d6e813c
3c17a1d
c0ca99f
442853b
0d9e65a
15f0614
fb8b27d
f444814
215e737
e75fc0f
eefb998
2a6c182
b8300b2
32e042b
53d4142
6e0502f
067371a
5780dcc
f31c206
ac21745
659aa43
0bd13f1
cb9db5e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -15,6 +15,21 @@ import ( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/thand-io/agent/internal/models" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Authentication Callback Handlers | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // This file implements two separate callback handlers: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 1. getAuthCallback() - OAuth2 GET callbacks | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // - Expects state and code in query parameters | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // - Used by OAuth2 providers (GitHub, Google, etc.) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 2. postAuthCallback() - SAML POST callbacks | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // - Expects RelayState and SAMLResponse in form parameters | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // - Supports SP-initiated (with RelayState) and IdP-initiated (no RelayState) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // - Used by SAML providers (Okta, Azure AD, etc.) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Both handlers delegate to getAuthCallbackPage() for session creation. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // getAuthRequest initiates the authentication flow | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // @Summary Initiate authentication | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -42,9 +57,28 @@ func (s *Server) getAuthRequest(c *gin.Context) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| config := s.GetConfig() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if len(callback) > 0 && strings.Compare(callback, config.GetLoginServerUrl()) == 0 { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| s.getErrorPage(c, http.StatusBadRequest, "Callback cannot be the login server") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Validate callback URL to prevent infinite loops | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Only block callbacks that would loop back to the auth request endpoint | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if len(callback) > 0 { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| callbackURL, callbackErr := url.Parse(callback) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| loginServerURL, loginServerErr := url.Parse(config.GetLoginServerUrl()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if callbackErr == nil && loginServerErr == nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Block if it's the same host and the callback would loop back to auth endpoints | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if callbackURL.Host == loginServerURL.Host && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (strings.HasPrefix(callbackURL.Path, "/api/v1/auth/request") || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| strings.HasPrefix(callbackURL.Path, "/api/v1/auth/callback")) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| s.getErrorPage(c, http.StatusBadRequest, "Callback cannot be the auth request or callback endpoint - this would create an infinite loop") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Only block callbacks that would loop back to the auth request endpoint | |
| if len(callback) > 0 { | |
| callbackURL, callbackErr := url.Parse(callback) | |
| loginServerURL, loginServerErr := url.Parse(config.GetLoginServerUrl()) | |
| if callbackErr == nil && loginServerErr == nil { | |
| // Block if it's the same host and the callback would loop back to auth endpoints | |
| if callbackURL.Host == loginServerURL.Host && | |
| (strings.HasPrefix(callbackURL.Path, "/api/v1/auth/request") || | |
| strings.HasPrefix(callbackURL.Path, "/api/v1/auth/callback")) { | |
| s.getErrorPage(c, http.StatusBadRequest, "Callback cannot be the auth request or callback endpoint - this would create an infinite loop") | |
| // Only block callbacks that would loop back to the exact same auth request endpoint (path and query) | |
| if len(callback) > 0 { | |
| callbackURL, callbackErr := url.Parse(callback) | |
| loginServerURL, loginServerErr := url.Parse(config.GetLoginServerUrl()) | |
| if callbackErr == nil && loginServerErr == nil { | |
| // Block if it's the same host, path, and query as the current request (i.e., would cause a loop) | |
| currentPath := c.Request.URL.Path | |
| currentRawQuery := c.Request.URL.RawQuery | |
| // Remove the "callback" parameter from the current query for comparison | |
| currentQueryVals := c.Request.URL.Query() | |
| currentQueryVals.Del("callback") | |
| currentQuery := currentQueryVals.Encode() | |
| callbackPath := callbackURL.Path | |
| callbackRawQuery := callbackURL.RawQuery | |
| callbackQueryVals := callbackURL.Query() | |
| callbackQueryVals.Del("callback") | |
| callbackQuery := callbackQueryVals.Encode() | |
| if callbackURL.Host == loginServerURL.Host && | |
| callbackPath == currentPath && | |
| callbackQuery == currentQuery { | |
| s.getErrorPage(c, http.StatusBadRequest, "Callback cannot be the same as the current auth request endpoint - this would create an infinite loop") |
willesq marked this conversation as resolved.
Show resolved
Hide resolved
willesq marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Dec 12, 2025
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.
The IdP-initiated SAML flow accepts authentication without validating a RelayState, which could potentially be exploited for session fixation or CSRF attacks. While the code includes security logging, consider adding additional validation such as checking for expected SAML assertion conditions (e.g., validating the Destination field matches the ACS URL, checking for replay attacks by tracking assertion IDs). Additionally, consider making IdP-initiated flows opt-in via configuration rather than always allowing them.
Uh oh!
There was an error while loading. Please reload this page.