-
-
Notifications
You must be signed in to change notification settings - Fork 809
OIDC Server #941
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: master
Are you sure you want to change the base?
OIDC Server #941
Changes from all commits
86df116
e7234fd
0efef49
1774781
862fa3b
9a51dd7
e072c64
c92b130
9ca2623
7f6810e
83895b5
44bda75
c463d98
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 |
|---|---|---|
| @@ -0,0 +1,362 @@ | ||
| package api | ||
|
|
||
| import ( | ||
| "context" | ||
| "crypto/rand" | ||
| "encoding/hex" | ||
| "errors" | ||
| "fmt" | ||
| "log" | ||
| "net/http" | ||
| "strings" | ||
| "sync" | ||
| "time" | ||
|
|
||
| "github.com/gin-gonic/gin" | ||
| "github.com/gotify/server/v2/auth" | ||
| "github.com/gotify/server/v2/config" | ||
| "github.com/gotify/server/v2/database" | ||
| "github.com/gotify/server/v2/model" | ||
| "github.com/zitadel/oidc/v3/pkg/client/rp" | ||
| httphelper "github.com/zitadel/oidc/v3/pkg/http" | ||
| "github.com/zitadel/oidc/v3/pkg/oidc" | ||
| ) | ||
|
|
||
| func NewOIDC(conf *config.Configuration, db *database.GormDatabase, userChangeNotifier *UserChangeNotifier) *OIDCAPI { | ||
| scopes := conf.OIDC.Scopes | ||
| if len(scopes) == 0 { | ||
| scopes = []string{"openid", "profile", "email"} | ||
| } | ||
|
|
||
| cookieKey := make([]byte, 32) | ||
| if _, err := rand.Read(cookieKey); err != nil { | ||
| log.Fatalf("failed to generate OIDC cookie key: %v", err) | ||
| } | ||
| cookieHandlerOpt := []httphelper.CookieHandlerOpt{} | ||
| if !conf.Server.SecureCookie { | ||
| cookieHandlerOpt = append(cookieHandlerOpt, httphelper.WithUnsecure()) | ||
| } | ||
| cookieHandler := httphelper.NewCookieHandler(cookieKey, cookieKey, cookieHandlerOpt...) | ||
|
|
||
| opts := []rp.Option{rp.WithCookieHandler(cookieHandler), rp.WithPKCE(cookieHandler)} | ||
|
|
||
| provider, err := rp.NewRelyingPartyOIDC( | ||
| context.Background(), | ||
| conf.OIDC.Issuer, | ||
| conf.OIDC.ClientID, | ||
| conf.OIDC.ClientSecret, | ||
| conf.OIDC.RedirectURL, | ||
| scopes, | ||
| opts..., | ||
| ) | ||
| if err != nil { | ||
| log.Fatalf("failed to initialize OIDC provider: %v", err) | ||
| } | ||
|
|
||
| return &OIDCAPI{ | ||
| DB: db, | ||
| Provider: provider, | ||
| UserChangeNotifier: userChangeNotifier, | ||
| UsernameClaim: conf.OIDC.UsernameClaim, | ||
| PasswordStrength: conf.PassStrength, | ||
| SecureCookie: conf.Server.SecureCookie, | ||
| AutoRegister: conf.OIDC.AutoRegister, | ||
| pendingSessions: make(map[string]*pendingOIDCSession), | ||
| } | ||
| } | ||
|
|
||
| const pendingSessionMaxAge = 10 * time.Minute | ||
|
|
||
| type pendingOIDCSession struct { | ||
| RedirectURI string | ||
| ClientName string | ||
| CreatedAt time.Time | ||
| } | ||
|
|
||
| // OIDCAPI provides handlers for OIDC authentication. | ||
| type OIDCAPI struct { | ||
| DB *database.GormDatabase | ||
| Provider rp.RelyingParty | ||
| UserChangeNotifier *UserChangeNotifier | ||
| UsernameClaim string | ||
| PasswordStrength int | ||
| SecureCookie bool | ||
| AutoRegister bool | ||
| pendingSessions map[string]*pendingOIDCSession | ||
| pendingSessionsMu sync.Mutex | ||
| } | ||
|
|
||
| func (a *OIDCAPI) storePendingSession(state string, session *pendingOIDCSession) { | ||
| a.pendingSessionsMu.Lock() | ||
| defer a.pendingSessionsMu.Unlock() | ||
| for s, sess := range a.pendingSessions { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel this could be done better, this has O(n^2) complexity over number of users who are authenticating at the same time. If we are unconditionally iterating through the entire map as the first action of the flow anyways, might as well just use an array (with swap-delete). A more complex solution is bulk cleanup like this: https://gist.github.com/eternal-flame-AD/e4a47a8a7e6b26bba365a7d24ab84221
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My plan was to have a simple implementation as we probably don't have many parallel logins anyway. Your decay map doesn't seem to complex, I'll use it, as it's not that much code overhead. |
||
| if time.Since(sess.CreatedAt) > pendingSessionMaxAge { | ||
| delete(a.pendingSessions, s) | ||
| } | ||
| } | ||
| a.pendingSessions[state] = session | ||
| } | ||
|
|
||
| func (a *OIDCAPI) popPendingSession(state string) (*pendingOIDCSession, bool) { | ||
| a.pendingSessionsMu.Lock() | ||
| session, ok := a.pendingSessions[state] | ||
| if ok { | ||
| delete(a.pendingSessions, state) | ||
| } | ||
| a.pendingSessionsMu.Unlock() | ||
| if !ok || time.Since(session.CreatedAt) > pendingSessionMaxAge { | ||
| return nil, false | ||
| } | ||
| return session, true | ||
| } | ||
|
|
||
| // swagger:operation GET /auth/oidc/login oidc oidcLogin | ||
| // | ||
| // Start the OIDC login flow (browser). | ||
| // | ||
| // Redirects the user to the OIDC provider's authorization endpoint. | ||
| // After authentication, the provider redirects back to the callback endpoint. | ||
| // | ||
| // --- | ||
| // parameters: | ||
| // - name: name | ||
| // in: query | ||
| // description: the client name to create after login | ||
| // required: true | ||
| // type: string | ||
| // responses: | ||
| // 302: | ||
| // description: Redirect to OIDC provider | ||
| // default: | ||
| // description: Error | ||
| // schema: | ||
| // $ref: "#/definitions/Error" | ||
| func (a *OIDCAPI) LoginHandler() gin.HandlerFunc { | ||
| return gin.WrapF(func(w http.ResponseWriter, r *http.Request) { | ||
| clientName := r.URL.Query().Get("name") | ||
| if clientName == "" { | ||
| http.Error(w, "invalid client name", http.StatusBadRequest) | ||
| return | ||
| } | ||
| state, err := a.generateState(clientName) | ||
| if err != nil { | ||
| http.Error(w, fmt.Sprintf("failed to generate state: %v", err), http.StatusInternalServerError) | ||
| return | ||
| } | ||
| rp.AuthURLHandler(func() string { return state }, a.Provider)(w, r) | ||
| }) | ||
| } | ||
|
|
||
| // swagger:operation GET /auth/oidc/callback oidc oidcCallback | ||
| // | ||
| // Handle the OIDC provider callback (browser). | ||
| // | ||
| // Exchanges the authorization code for tokens, resolves the user, | ||
| // creates a gotify client, sets a session cookie, and redirects to the UI. | ||
| // | ||
| // --- | ||
| // parameters: | ||
| // - name: code | ||
| // in: query | ||
| // description: the authorization code from the OIDC provider | ||
| // required: true | ||
| // type: string | ||
| // - name: state | ||
| // in: query | ||
| // description: the state parameter for CSRF protection | ||
| // required: true | ||
| // type: string | ||
| // responses: | ||
| // 307: | ||
| // description: Redirect to UI | ||
| // default: | ||
| // description: Error | ||
| // schema: | ||
| // $ref: "#/definitions/Error" | ||
| func (a *OIDCAPI) CallbackHandler() gin.HandlerFunc { | ||
| callback := func(w http.ResponseWriter, r *http.Request, tokens *oidc.Tokens[*oidc.IDTokenClaims], state string, provider rp.RelyingParty, info *oidc.UserInfo) { | ||
| user, status, err := a.resolveUser(info) | ||
| if err != nil { | ||
| http.Error(w, err.Error(), status) | ||
| return | ||
| } | ||
| clientName, _, _ := strings.Cut(state, ":") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can the name itself already have a colon in it?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True, I'll reverse the order of the random nonce and the client name and then split by the first colon as the nonce is hex encoded and never includes a colon. |
||
| client, err := a.createClient(clientName, user.ID) | ||
| if err != nil { | ||
| http.Error(w, fmt.Sprintf("failed to create client: %v", err), http.StatusInternalServerError) | ||
| return | ||
| } | ||
| auth.SetCookie(w, client.Token, auth.CookieMaxAge, a.SecureCookie) | ||
| // A reverse proxy may have already stripped a url prefix from the URL | ||
| // without us knowing, we have to make a relative redirect. | ||
| // We cannot use http.Redirect as this normalizes the Path with r.URL. | ||
| w.Header().Set("Location", "../../") | ||
| w.WriteHeader(http.StatusTemporaryRedirect) | ||
| } | ||
| return gin.WrapF(rp.CodeExchangeHandler(rp.UserinfoCallback(callback), a.Provider)) | ||
| } | ||
|
|
||
| // swagger:operation POST /auth/oidc/external/authorize oidc externalAuthorize | ||
| // | ||
| // Initiate the OIDC authorization flow for a native app. | ||
| // | ||
| // The app generates a PKCE code_verifier and code_challenge, then calls this | ||
| // endpoint. The server forwards the code_challenge to the OIDC provider and | ||
| // returns the authorization URL for the app to open in a browser. | ||
| // | ||
| // --- | ||
| // consumes: [application/json] | ||
| // produces: [application/json] | ||
| // parameters: | ||
| // - name: body | ||
| // in: body | ||
| // required: true | ||
| // schema: | ||
| // $ref: "#/definitions/OIDCExternalAuthorizeRequest" | ||
| // responses: | ||
| // 200: | ||
| // description: Ok | ||
| // schema: | ||
| // $ref: "#/definitions/OIDCExternalAuthorizeResponse" | ||
| // default: | ||
| // description: Error | ||
| // schema: | ||
| // $ref: "#/definitions/Error" | ||
| func (a *OIDCAPI) ExternalAuthorizeHandler(ctx *gin.Context) { | ||
| var req model.OIDCExternalAuthorizeRequest | ||
| if err := ctx.ShouldBindJSON(&req); err != nil { | ||
| ctx.AbortWithError(http.StatusBadRequest, err) | ||
| return | ||
| } | ||
| state, err := a.generateState(req.Name) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this how it works for VaultWarden (server generates state) ? I'm not super sure what this is for. From my cursory exploration it seems like in VaultWarden state is simply for the client's convenience (might be wrong..)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay this seems to be documented in RFC6749, I got it messed up. The flow is right although the bookkeeping ClientName could be clearer (don't keep them both in the server memory and in the token text at the same time).
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, this is different from vaultwarden, but as I understood is this for CSRF protection, so basically the android app has to check the initial state against the state that will be received on the redirect uri. So I don't think it matters. Do you mean removing the client name from the pending session struct and then directly reading it from the state similar to how it's done in the web ui flow? |
||
| if err != nil { | ||
| ctx.AbortWithError(http.StatusInternalServerError, err) | ||
| return | ||
| } | ||
| a.storePendingSession(state, &pendingOIDCSession{ | ||
| RedirectURI: req.RedirectURI, ClientName: req.Name, CreatedAt: time.Now(), | ||
| }) | ||
| authOpts := []rp.AuthURLOpt{ | ||
| rp.AuthURLOpt(rp.WithURLParam("redirect_uri", req.RedirectURI)), | ||
| rp.WithCodeChallenge(req.CodeChallenge), | ||
| } | ||
| ctx.JSON(http.StatusOK, &model.OIDCExternalAuthorizeResponse{ | ||
| AuthorizeURL: rp.AuthURL(state, a.Provider, authOpts...), | ||
| State: state, | ||
| }) | ||
| } | ||
|
|
||
| // swagger:operation POST /auth/oidc/external/token oidc externalToken | ||
| // | ||
| // Exchange an authorization code for a gotify client token. | ||
| // | ||
| // After the user authenticates with the OIDC provider and the app receives | ||
| // the authorization code via redirect, the app calls this endpoint with the | ||
| // code and PKCE code_verifier. The server exchanges the code with the OIDC | ||
| // provider and returns a gotify client token. | ||
| // | ||
| // --- | ||
| // consumes: [application/json] | ||
| // produces: [application/json] | ||
| // parameters: | ||
| // - name: body | ||
| // in: body | ||
| // required: true | ||
| // schema: | ||
| // $ref: "#/definitions/OIDCExternalTokenRequest" | ||
| // responses: | ||
| // 200: | ||
| // description: Ok | ||
| // schema: | ||
| // $ref: "#/definitions/OIDCExternalTokenResponse" | ||
| // default: | ||
| // description: Error | ||
| // schema: | ||
| // $ref: "#/definitions/Error" | ||
| func (a *OIDCAPI) ExternalTokenHandler(ctx *gin.Context) { | ||
| var req model.OIDCExternalTokenRequest | ||
| if err := ctx.ShouldBindJSON(&req); err != nil { | ||
| ctx.AbortWithError(http.StatusBadRequest, err) | ||
| return | ||
| } | ||
| session, ok := a.popPendingSession(req.State) | ||
| if !ok { | ||
| ctx.AbortWithError(http.StatusBadRequest, errors.New("unknown or expired state")) | ||
| return | ||
| } | ||
| exchangeOpts := []rp.CodeExchangeOpt{ | ||
| rp.CodeExchangeOpt(rp.WithURLParam("redirect_uri", session.RedirectURI)), | ||
| rp.WithCodeVerifier(req.CodeVerifier), | ||
| } | ||
| tokens, err := rp.CodeExchange[*oidc.IDTokenClaims](ctx.Request.Context(), req.Code, a.Provider, exchangeOpts...) | ||
| if err != nil { | ||
| ctx.AbortWithError(http.StatusUnauthorized, fmt.Errorf("token exchange failed: %w", err)) | ||
| return | ||
| } | ||
| info, err := rp.Userinfo[*oidc.UserInfo](ctx.Request.Context(), tokens.AccessToken, tokens.TokenType, tokens.IDTokenClaims.GetSubject(), a.Provider) | ||
| if err != nil { | ||
| ctx.AbortWithError(http.StatusInternalServerError, fmt.Errorf("failed to get user info: %w", err)) | ||
| return | ||
| } | ||
| user, status, resolveErr := a.resolveUser(info) | ||
| if resolveErr != nil { | ||
| ctx.AbortWithError(status, resolveErr) | ||
| return | ||
| } | ||
| client, err := a.createClient(session.ClientName, user.ID) | ||
| if err != nil { | ||
| ctx.AbortWithError(http.StatusInternalServerError, err) | ||
| return | ||
| } | ||
| ctx.JSON(http.StatusOK, &model.OIDCExternalTokenResponse{ | ||
| Token: client.Token, | ||
| User: &model.UserExternal{ID: user.ID, Name: user.Name, Admin: user.Admin}, | ||
| }) | ||
| } | ||
|
|
||
| func (a *OIDCAPI) generateState(name string) (string, error) { | ||
| nonce := make([]byte, 20) | ||
| if _, err := rand.Read(nonce); err != nil { | ||
| return "", err | ||
| } | ||
| return name + ":" + hex.EncodeToString(nonce), nil | ||
| } | ||
|
|
||
| // resolveUser looks up or creates a user from OIDC userinfo claims. | ||
| func (a *OIDCAPI) resolveUser(info *oidc.UserInfo) (*model.User, int, error) { | ||
| usernameRaw, ok := info.Claims[a.UsernameClaim] | ||
| if !ok { | ||
| return nil, http.StatusInternalServerError, fmt.Errorf("username claim %q is missing", a.UsernameClaim) | ||
| } | ||
| username := fmt.Sprint(usernameRaw) | ||
| if username == "" || usernameRaw == nil { | ||
| return nil, http.StatusInternalServerError, fmt.Errorf("username claim was empty") | ||
| } | ||
|
|
||
| user, err := a.DB.GetUserByName(username) | ||
| if err != nil { | ||
| return nil, http.StatusInternalServerError, fmt.Errorf("database error: %w", err) | ||
| } | ||
| if user == nil { | ||
| if !a.AutoRegister { | ||
| return nil, http.StatusForbidden, fmt.Errorf("user does not exist and auto-registration is disabled") | ||
| } | ||
| user = &model.User{Name: username, Admin: false, Pass: nil} | ||
| if err := a.DB.CreateUser(user); err != nil { | ||
| return nil, http.StatusInternalServerError, fmt.Errorf("failed to create user: %w", err) | ||
| } | ||
| if err := a.UserChangeNotifier.fireUserAdded(user.ID); err != nil { | ||
| log.Printf("Could not notify user change: %v\n", err) | ||
| } | ||
| } | ||
| return user, 0, nil | ||
| } | ||
|
|
||
| func (a *OIDCAPI) createClient(name string, userID uint) (*model.Client, error) { | ||
| client := &model.Client{ | ||
| Name: name, | ||
| Token: auth.GenerateNotExistingToken(generateClientToken, func(t string) bool { c, _ := a.DB.GetClientByToken(t); return c != nil }), | ||
| UserID: userID, | ||
| } | ||
| return client, a.DB.CreateClient(client) | ||
| } | ||
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.
@eternal-flame-AD sorry this is kinda big, but would you mind having a look at this? Don't worry about declining if you don't want to review this, or don't have the time. I don't want to burden you with this :D.
Anyway, my plan is to create a snapshot docker build in the coming days and then let the people in #433 try out the functionality to improve the user-guide and find possible bugs with special IdP servers. So there is no rush reviewing this.
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.
Sure, I can look at this tonight first to see if there are large architectural/security changes needed and we can do a final round for final check and nitpicks after the code settles. Probably won't review Android though because I'm not very familar with android dev.
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.
Awesome thanks!