Skip to content

Commit fb6f90b

Browse files
committed
fix: don't store client name in state
1 parent 1250e12 commit fb6f90b

2 files changed

Lines changed: 13 additions & 9 deletions

File tree

api/oidc.go

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"fmt"
99
"log"
1010
"net/http"
11-
"strings"
1211
"sync"
1312
"time"
1413

@@ -138,11 +137,12 @@ func (a *OIDCAPI) LoginHandler() gin.HandlerFunc {
138137
http.Error(w, "invalid client name", http.StatusBadRequest)
139138
return
140139
}
141-
state, err := a.generateState(clientName)
140+
state, err := a.generateState()
142141
if err != nil {
143142
http.Error(w, fmt.Sprintf("failed to generate state: %v", err), http.StatusInternalServerError)
144143
return
145144
}
145+
a.storePendingSession(state, &pendingOIDCSession{ClientName: clientName, CreatedAt: time.Now()})
146146
rp.AuthURLHandler(func() string { return state }, a.Provider)(w, r)
147147
})
148148
}
@@ -180,8 +180,12 @@ func (a *OIDCAPI) CallbackHandler() gin.HandlerFunc {
180180
http.Error(w, err.Error(), status)
181181
return
182182
}
183-
clientName, _, _ := strings.Cut(state, ":")
184-
client, err := a.createClient(clientName, user.ID)
183+
session, ok := a.popPendingSession(state)
184+
if !ok {
185+
http.Error(w, "unknown or expired state", http.StatusBadRequest)
186+
return
187+
}
188+
client, err := a.createClient(session.ClientName, user.ID)
185189
if err != nil {
186190
http.Error(w, fmt.Sprintf("failed to create client: %v", err), http.StatusInternalServerError)
187191
return
@@ -228,7 +232,7 @@ func (a *OIDCAPI) ExternalAuthorizeHandler(ctx *gin.Context) {
228232
ctx.AbortWithError(http.StatusBadRequest, err)
229233
return
230234
}
231-
state, err := a.generateState(req.Name)
235+
state, err := a.generateState()
232236
if err != nil {
233237
ctx.AbortWithError(http.StatusInternalServerError, err)
234238
return
@@ -314,12 +318,12 @@ func (a *OIDCAPI) ExternalTokenHandler(ctx *gin.Context) {
314318
})
315319
}
316320

317-
func (a *OIDCAPI) generateState(name string) (string, error) {
321+
func (a *OIDCAPI) generateState() (string, error) {
318322
nonce := make([]byte, 20)
319323
if _, err := rand.Read(nonce); err != nil {
320324
return "", err
321325
}
322-
return name + ":" + hex.EncodeToString(nonce), nil
326+
return hex.EncodeToString(nonce), nil
323327
}
324328

325329
// resolveUser looks up or creates a user from OIDC userinfo claims.

api/oidc_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ func (s *OIDCSuite) AfterTest(suiteName, testName string) {
5555
}
5656

5757
func (s *OIDCSuite) Test_GenerateState_Unique() {
58-
s1, _ := s.a.generateState("app")
59-
s2, _ := s.a.generateState("app")
58+
s1, _ := s.a.generateState()
59+
s2, _ := s.a.generateState()
6060
assert.NotEqual(s.T(), s1, s2)
6161
}
6262

0 commit comments

Comments
 (0)