0645e19 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Cleanup internalSession params & handle profileURL Bearer case better

`findClaimsFromIDToken` would always have a `nil` access token and not be able to hit the userinfo endpoint in Bearer case. If access token is nil, default to legacy `session.Email = claim.Subject` that all JWT bearers used to have, even if a valid profileURL is present.

commit0645e19c240984259d0d0a44ef46f8c24db9530c authorNick Meves <nick.meves@greenhouse.io> dateSun 9 Aug 2020, 18:18 (6 years ago) parentdcc7541

2 files changed+26−37

compare with master

unifiedsplit

providers/oidc.go+18−16 providers/oidc_test.go+8−21

providers/oidc.go+18−16

@@ -157,7 +157,7 @@ func (p *OIDCProvider) createSessionState(ctx context.Context, token *oauth2.Tok

157157 newSession = &sessions.SessionState{}

158158 } else {

159159 var err error

160− newSession, err = p.createSessionStateInternal(ctx, token.Extra("id_token").(string), idToken, token, false)

160+ newSession, err = p.createSessionStateInternal(ctx, idToken, token)

161161 if err != nil {

162162 return nil, err

163163 }

@@ -172,7 +172,7 @@ func (p *OIDCProvider) createSessionState(ctx context.Context, token *oauth2.Tok

172172}

173173

174174func (p *OIDCProvider) CreateSessionStateFromBearerToken(ctx context.Context, rawIDToken string, idToken *oidc.IDToken) (*sessions.SessionState, error) {

175− newSession, err := p.createSessionStateInternal(ctx, rawIDToken, idToken, nil, true)

175+ newSession, err := p.createSessionStateInternal(ctx, idToken, nil)

176176 if err != nil {

177177 return nil, err

178178 }

@@ -185,24 +185,22 @@ func (p *OIDCProvider) CreateSessionStateFromBearerToken(ctx context.Context, ra

185185 return newSession, nil

186186}

187187

188−func (p *OIDCProvider) createSessionStateInternal(ctx context.Context, rawIDToken string, idToken *oidc.IDToken, token *oauth2.Token, bearer bool) (*sessions.SessionState, error) {

188+func (p *OIDCProvider) createSessionStateInternal(ctx context.Context, idToken *oidc.IDToken, token *oauth2.Token) (*sessions.SessionState, error) {

189189

190190 newSession := &sessions.SessionState{}

191191

192192 if idToken == nil {

193193 return newSession, nil

194194 }

195− accessToken := ""

196− if token != nil {

197− accessToken = token.AccessToken

198− }

199195

200− claims, err := p.findClaimsFromIDToken(ctx, idToken, accessToken, p.ProfileURL.String(), bearer)

196+ claims, err := p.findClaimsFromIDToken(ctx, idToken, token)

201197 if err != nil {

202198 return nil, fmt.Errorf("couldn't extract claims from id_token (%v)", err)

203199 }

204200

205− newSession.IDToken = rawIDToken

201+ if token != nil {

202+ newSession.IDToken = token.Extra("id_token").(string)

203+ }

206204

207205 newSession.Email = claims.UserID // TODO Rename SessionState.Email to .UserID in the near future

208206

@@ -230,7 +228,7 @@ func getOIDCHeader(accessToken string) http.Header {

230228 return header

231229}

232230

233−func (p *OIDCProvider) findClaimsFromIDToken(ctx context.Context, idToken *oidc.IDToken, accessToken string, profileURL string, bearer bool) (*OIDCClaims, error) {

231+func (p *OIDCProvider) findClaimsFromIDToken(ctx context.Context, idToken *oidc.IDToken, token *oauth2.Token) (*OIDCClaims, error) {

234232 claims := &OIDCClaims{}

235233 // Extract default claims.

236234 if err := idToken.Claims(&claims); err != nil {

@@ -248,11 +246,15 @@ func (p *OIDCProvider) findClaimsFromIDToken(ctx context.Context, idToken *oidc.

248246

249247 // userID claim was not present or was empty in the ID Token

250248 if claims.UserID == "" {

251− if profileURL == "" {

252− if bearer {

253− claims.UserID = claims.Subject

254− return claims, nil

255− }

249+ // BearerToken case, allow empty UserID

250+ // ProfileURL checks below won't work since we don't have an access token

251+ if token == nil {

252+ claims.UserID = claims.Subject

253+ return claims, nil

254+ }

255+

256+ profileURL := p.ProfileURL.String()

257+ if profileURL == "" || token.AccessToken == "" {

256258 return nil, fmt.Errorf("id_token did not contain user ID claim (%q)", p.UserIDClaim)

257259 }

258260

@@ -261,7 +263,7 @@ func (p *OIDCProvider) findClaimsFromIDToken(ctx context.Context, idToken *oidc.

261263 // Make a query to the userinfo endpoint, and attempt to locate the email from there.

262264 respJSON, err := requests.New(profileURL).

263265 WithContext(ctx).

264− WithHeaders(getOIDCHeader(accessToken)).

266+ WithHeaders(getOIDCHeader(token.AccessToken)).

265267 Do().

266268 UnmarshalJSON()

267269 if err != nil {

providers/oidc_test.go+8−21

@@ -274,23 +274,18 @@ func TestCreateSessionStateFromBearerToken(t *testing.T) {

274274

275275 testCases := map[string]struct {

276276 IDToken idTokenClaims

277− ProfileURL bool

277+ ExpectedUser string

278278 ExpectedEmail string

279279 }{

280280 "Default IDToken": {

281281 IDToken: defaultIDToken,

282− ProfileURL: true,

283− ExpectedEmail: profileURLEmail,

282+ ExpectedUser: defaultIDToken.Subject,

283+ ExpectedEmail: defaultIDToken.Email,

284284 },

285− "Minimal IDToken with no OIDC Profile URL": {

285+ "Minimal IDToken with no email claim": {

286286 IDToken: minimalIDToken,

287− ProfileURL: false,

288− ExpectedEmail: "",

289− },

290− "Minimal IDToken with OIDC Profile URL": {

291− IDToken: minimalIDToken,

292− ProfileURL: true,

293− ExpectedEmail: profileURLEmail,

287+ ExpectedUser: minimalIDToken.Subject,

288+ ExpectedEmail: minimalIDToken.Subject,

294289 },

295290 }

296291 for testName, tc := range testCases {

@@ -298,9 +293,6 @@ func TestCreateSessionStateFromBearerToken(t *testing.T) {

298293 jsonResp := []byte(fmt.Sprintf(`{"email":"%s"}`, profileURLEmail))

299294 server, provider := newTestSetup(jsonResp)

300295 defer server.Close()

301− if !tc.ProfileURL {

302− provider.ProfileURL = &url.URL{}

303− }

304296

305297 rawIDToken, err := newSignedTestIDToken(tc.IDToken)

306298 assert.NoError(t, err)

@@ -315,13 +307,8 @@ func TestCreateSessionStateFromBearerToken(t *testing.T) {

315307 ss, err := provider.CreateSessionStateFromBearerToken(context.Background(), rawIDToken, idToken)

316308 assert.NoError(t, err)

317309

318− if tc.ExpectedEmail != "" {

319− assert.Equal(t, tc.ExpectedEmail, ss.Email)

320− assert.NotEqual(t, ss.Email, ss.User)

321− } else {

322− assert.Equal(t, tc.IDToken.Subject, ss.Email)

323− assert.Equal(t, ss.Email, ss.User)

324− }

310+ assert.Equal(t, tc.ExpectedUser, ss.User)

311+ assert.Equal(t, tc.ExpectedEmail, ss.Email)

325312 assert.Equal(t, rawIDToken, ss.IDToken)

326313 assert.Equal(t, rawIDToken, ss.AccessToken)

327314 assert.Equal(t, "", ss.RefreshToken)