1b3b004 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Streamline ErrMissingCode in provider Redeem methods

commit1b3b00443ae6b4a56f92cede54433e1c558afc2e authorNick Meves <nick.meves@greenhouse.io> dateFri 23 Oct 2020, 19:35 (6 years ago) parentb92fd4b

6 files changed+36−37

compare with master

unifiedsplit

oauthproxy.go+1−1 oauthproxy_test.go+5 providers/azure.go+6−8 providers/google.go+1−2 providers/logingov.go+7−10 providers/provider_default.go+16−16

oauthproxy.go+1−1

@@ -394,7 +394,7 @@ func (p *OAuthProxy) GetRedirectURI(host string) string {

394394

395395func (p *OAuthProxy) redeemCode(ctx context.Context, host, code string) (*sessionsapi.SessionState, error) {

396396 if code == "" {

397− return nil, errors.New("missing code")

397+ return nil, providers.ErrMissingCode

398398 }

399399 redirectURI := p.GetRedirectURI(host)

400400 s, err := p.provider.Redeem(ctx, redirectURI, code)

oauthproxy_test.go+5

@@ -1133,6 +1133,11 @@ func TestUserInfoEndpointAccepted(t *testing.T) {

11331133 Email: "john.doe@example.com", AccessToken: "my_access_token"}

11341134 err = test.SaveSession(startSession)

11351135 assert.NoError(t, err)

1136+

1137+ test.proxy.ServeHTTP(test.rw, test.req)

1138+ assert.Equal(t, http.StatusOK, test.rw.Code)

1139+ bodyBytes, _ := ioutil.ReadAll(test.rw.Body)

1140+ assert.Equal(t, "{\"email\":\"john.doe@example.com\"}\n", string(bodyBytes))

11361141}

11371142

11381143func TestUserInfoEndpointUnauthorizedOnNoCookieSetError(t *testing.T) {

providers/azure.go+6−8

@@ -108,14 +108,13 @@ func overrideTenantURL(current, defaultURL *url.URL, tenant, path string) {

108108}

109109

110110// Redeem exchanges the OAuth2 authentication token for an ID token

111−func (p *AzureProvider) Redeem(ctx context.Context, redirectURL, code string) (s *sessions.SessionState, err error) {

111+func (p *AzureProvider) Redeem(ctx context.Context, redirectURL, code string) (*sessions.SessionState, error) {

112112 if code == "" {

113− err = errors.New("missing code")

114− return

113+ return nil, ErrMissingCode

115114 }

116115 clientSecret, err := p.GetClientSecret()

117116 if err != nil {

118− return

117+ return nil, err

119118 }

120119

121120 params := url.Values{}

@@ -149,15 +148,14 @@ func (p *AzureProvider) Redeem(ctx context.Context, redirectURL, code string) (s

149148

150149 created := time.Now()

151150 expires := time.Unix(jsonResponse.ExpiresOn, 0)

152− s = &sessions.SessionState{

151+

152+ return &sessions.SessionState{

153153 AccessToken: jsonResponse.AccessToken,

154154 IDToken: jsonResponse.IDToken,

155155 CreatedAt: &created,

156156 ExpiresOn: &expires,

157157 RefreshToken: jsonResponse.RefreshToken,

158− }

159− return

160−

158+ }, nil

161159}

162160

163161// RefreshSessionIfNeeded checks if the session has expired and uses the

providers/google.go+1−2

@@ -126,8 +126,7 @@ func claimsFromIDToken(idToken string) (*claims, error) {

126126// Redeem exchanges the OAuth2 authentication token for an ID token

127127func (p *GoogleProvider) Redeem(ctx context.Context, redirectURL, code string) (*sessions.SessionState, error) {

128128 if code == "" {

129− err := errors.New("missing code")

130− return nil, err

129+ return nil, ErrMissingCode

131130 }

132131 clientSecret, err := p.GetClientSecret()

133132 if err != nil {

providers/logingov.go+7−10

@@ -4,7 +4,6 @@ import (

44 "bytes"

55 "context"

66 "crypto/rsa"

7− "errors"

87 "fmt"

98 "math/rand"

109 "net/url"

@@ -153,10 +152,9 @@ func emailFromUserInfo(ctx context.Context, accessToken string, userInfoEndpoint

153152}

154153

155154// Redeem exchanges the OAuth2 authentication token for an ID token

156−func (p *LoginGovProvider) Redeem(ctx context.Context, redirectURL, code string) (s *sessions.SessionState, err error) {

155+func (p *LoginGovProvider) Redeem(ctx context.Context, redirectURL, code string) (*sessions.SessionState, error) {

157156 if code == "" {

158− err = errors.New("missing code")

159− return

157+ return nil, ErrMissingCode

160158 }

161159

162160 claims := &jwt.StandardClaims{

@@ -169,7 +167,7 @@ func (p *LoginGovProvider) Redeem(ctx context.Context, redirectURL, code string)

169167 token := jwt.NewWithClaims(jwt.GetSigningMethod("RS256"), claims)

170168 ss, err := token.SignedString(p.JWTKey)

171169 if err != nil {

172− return

170+ return nil, err

173171 }

174172

175173 params := url.Values{}

@@ -199,28 +197,27 @@ func (p *LoginGovProvider) Redeem(ctx context.Context, redirectURL, code string)

199197 // check nonce here

200198 err = checkNonce(jsonResponse.IDToken, p)

201199 if err != nil {

202− return

200+ return nil, err

203201 }

204202

205203 // Get the email address

206204 var email string

207205 email, err = emailFromUserInfo(ctx, jsonResponse.AccessToken, p.ProfileURL.String())

208206 if err != nil {

209− return

207+ return nil, err

210208 }

211209

212210 created := time.Now()

213211 expires := time.Now().Add(time.Duration(jsonResponse.ExpiresIn) * time.Second).Truncate(time.Second)

214212

215213 // Store the data that we found in the session state

216− s = &sessions.SessionState{

214+ return &sessions.SessionState{

217215 AccessToken: jsonResponse.AccessToken,

218216 IDToken: jsonResponse.IDToken,

219217 CreatedAt: &created,

220218 ExpiresOn: &expires,

221219 Email: email,

222− }

223− return

220+ }, nil

224221}

225222

226223// GetLoginURL overrides GetLoginURL to add login.gov parameters

providers/provider_default.go+16−16

@@ -19,18 +19,21 @@ var (

1919 // implementation method that doesn't have sensible defaults

2020 ErrNotImplemented = errors.New("not implemented")

2121

22+ // ErrMissingCode is returned when a Redeem method is called with an empty

23+ // code

24+ ErrMissingCode = errors.New("missing code")

25+

2226 _ Provider = (*ProviderData)(nil)

2327)

2428

2529// Redeem provides a default implementation of the OAuth2 token redemption process

26−func (p *ProviderData) Redeem(ctx context.Context, redirectURL, code string) (s *sessions.SessionState, err error) {

30+func (p *ProviderData) Redeem(ctx context.Context, redirectURL, code string) (*sessions.SessionState, error) {

2731 if code == "" {

28− err = errors.New("missing code")

29− return

32+ return nil, ErrMissingCode

3033 }

3134 clientSecret, err := p.GetClientSecret()

3235 if err != nil {

33− return

36+ return nil, err

3437 }

3538

3639 params := url.Values{}

@@ -59,24 +62,21 @@ func (p *ProviderData) Redeem(ctx context.Context, redirectURL, code string) (s

5962 }

6063 err = result.UnmarshalInto(&jsonResponse)

6164 if err == nil {

62− s = &sessions.SessionState{

65+ return &sessions.SessionState{

6366 AccessToken: jsonResponse.AccessToken,

64− }

65− return

67+ }, nil

6668 }

6769

68− var v url.Values

69− v, err = url.ParseQuery(string(result.Body()))

70+ values, err := url.ParseQuery(string(result.Body()))

7071 if err != nil {

71− return

72+ return nil, err

7273 }

73− if a := v.Get("access_token"); a != "" {

74+ if token := values.Get("access_token"); token != "" {

7475 created := time.Now()

75− s = &sessions.SessionState{AccessToken: a, CreatedAt: &created}

76− } else {

77− err = fmt.Errorf("no access token found %s", result.Body())

76+ return &sessions.SessionState{AccessToken: token, CreatedAt: &created}, nil

7877 }

79− return

78+

79+ return nil, fmt.Errorf("no access token found %s", result.Body())

8080}

8181

8282// GetLoginURL with typical oauth parameters

@@ -100,7 +100,7 @@ func (p *ProviderData) EnrichSessionState(_ context.Context, _ *sessions.Session

100100

101101// Authorize performs global authorization on an authenticated session.

102102// This is not used for fine-grained per route authorization rules.

103−func (p *ProviderData) Authorize(ctx context.Context, s *sessions.SessionState) (bool, error) {

103+func (p *ProviderData) Authorize(_ context.Context, s *sessions.SessionState) (bool, error) {

104104 if len(p.AllowedGroups) == 0 {

105105 return true, nil

106106 }