From dd752eb7f4f93b00f5bd9d4464950f5294099976 Mon Sep 17 00:00:00 2001 From: Victor Kareh Date: Tue, 24 May 2022 16:20:26 -0400 Subject: [PATCH] authentication: Allow client credential grants with basic auth AWS Cognito user pools provide client credential grants using an basic Authorization header, rather than sending the client ID and secret in the request body. Since RH-SSO and Keycloak allow basic auth, this change allows OCM to support authenticating against either provider. --- authentication/transport_wrapper.go | 99 +++++++++++++----------- authentication/transport_wrapper_test.go | 53 ++++++++++++- 2 files changed, 107 insertions(+), 45 deletions(-) diff --git a/authentication/transport_wrapper.go b/authentication/transport_wrapper.go index 947e667f3..5f151b1d5 100644 --- a/authentication/transport_wrapper.go +++ b/authentication/transport_wrapper.go @@ -387,38 +387,41 @@ func (b *TransportWrapperBuilder) Build(ctx context.Context) (result *TransportW err = fmt.Errorf("claims of token %d are of type '%T'", i, claims) return } - claim, ok := claims["typ"] + claim, ok := claims["token_use"] if !ok { - // When the token doesn't have the `typ` claim we will use the position to - // decide: first token should be the access token and second should be the - // refresh token. That is consistent with the signature of the method that - // returns the tokens. - switch i { - case 0: - b.logger.Debug( - ctx, - "First token doesn't have a 'typ' claim, will assume "+ - "that it is an access token", - ) - accessToken = &tokenInfo{ - text: text, - object: object, + claim, ok = claims["typ"] + if !ok { + // When the token doesn't have the `typ` claim we will use the position to + // decide: first token should be the access token and second should be the + // refresh token. That is consistent with the signature of the method that + // returns the tokens. + switch i { + case 0: + b.logger.Debug( + ctx, + "First token doesn't have a 'typ' claim, will assume "+ + "that it is an access token", + ) + accessToken = &tokenInfo{ + text: text, + object: object, + } + continue + case 1: + b.logger.Debug( + ctx, + "Second token doesn't have a 'typ' claim, will assume "+ + "that it is a refresh token", + ) + refreshToken = &tokenInfo{ + text: text, + object: object, + } + continue + default: + err = fmt.Errorf("token %d doesn't contain the 'typ' claim", i) + return } - continue - case 1: - b.logger.Debug( - ctx, - "Second token doesn't have a 'typ' claim, will assume "+ - "that it is a refresh token", - ) - refreshToken = &tokenInfo{ - text: text, - object: object, - } - continue - default: - err = fmt.Errorf("token %d doesn't contain the 'typ' claim", i) - return } } typ, ok := claim.(string) @@ -427,7 +430,7 @@ func (b *TransportWrapperBuilder) Build(ctx context.Context) (result *TransportW return } switch strings.ToLower(typ) { - case "bearer": + case "access", "bearer": accessToken = &tokenInfo{ text: text, object: object, @@ -857,12 +860,17 @@ func (w *TransportWrapper) currentTokens() (access, refresh string) { func (w *TransportWrapper) sendClientCredentialsForm(ctx context.Context, attempt int) (code int, result *internal.TokenResponse, err error) { form := url.Values{} + headers := map[string]string{} w.logger.Debug(ctx, "Requesting new token using the client credentials grant") form.Set(grantTypeField, clientCredentialsGrant) form.Set(clientIDField, w.clientID) - form.Set(clientSecretField, w.clientSecret) form.Set(scopeField, strings.Join(w.scopes, " ")) - return w.sendForm(ctx, form, attempt) + // Encode client_id and client_secret to use as basic auth + // https://datatracker.ietf.org/doc/html/rfc6749#section-2.3.1 + auth := fmt.Sprintf("%s:%s", w.clientID, w.clientSecret) + hash := base64.StdEncoding.EncodeToString([]byte(auth)) + headers["Authorization"] = fmt.Sprintf("Basic %s", hash) + return w.sendForm(ctx, form, headers, attempt) } func (w *TransportWrapper) sendPasswordForm(ctx context.Context, attempt int) (code int, @@ -874,7 +882,7 @@ func (w *TransportWrapper) sendPasswordForm(ctx context.Context, attempt int) (c form.Set(usernameField, w.user) form.Set(passwordField, w.password) form.Set(scopeField, strings.Join(w.scopes, " ")) - return w.sendForm(ctx, form, attempt) + return w.sendForm(ctx, form, nil, attempt) } func (w *TransportWrapper) sendRefreshForm(ctx context.Context, attempt int) (code int, @@ -884,15 +892,15 @@ func (w *TransportWrapper) sendRefreshForm(ctx context.Context, attempt int) (co form.Set(grantTypeField, refreshTokenGrant) form.Set(clientIDField, w.clientID) form.Set(refreshTokenField, w.refreshToken.text) - code, result, err = w.sendForm(ctx, form, attempt) + code, result, err = w.sendForm(ctx, form, nil, attempt) return } -func (w *TransportWrapper) sendForm(ctx context.Context, form url.Values, +func (w *TransportWrapper) sendForm(ctx context.Context, form url.Values, headers map[string]string, attempt int) (code int, result *internal.TokenResponse, err error) { // Measure the time that it takes to send the request and receive the response: start := time.Now() - code, result, err = w.sendFormTimed(ctx, form) + code, result, err = w.sendFormTimed(ctx, form, headers) elapsed := time.Since(start) // Update the metrics: @@ -913,7 +921,7 @@ func (w *TransportWrapper) sendForm(ctx context.Context, form url.Values, return } -func (w *TransportWrapper) sendFormTimed(ctx context.Context, form url.Values) (code int, +func (w *TransportWrapper) sendFormTimed(ctx context.Context, form url.Values, headers map[string]string) (code int, result *internal.TokenResponse, err error) { // Create the HTTP request: body := []byte(form.Encode()) @@ -925,6 +933,10 @@ func (w *TransportWrapper) sendFormTimed(ctx context.Context, form url.Values) ( } header.Set("Content-Type", "application/x-www-form-urlencoded") header.Set("Accept", "application/json") + // Add any additional headers: + for k, v := range headers { + header.Set(k, v) + } if err != nil { err = fmt.Errorf("can't create request: %w", err) return @@ -1012,15 +1024,15 @@ func (w *TransportWrapper) sendFormTimed(ctx context.Context, form url.Values) ( } } - // The refresh token isn't mandatory for the password and client credentials grants: + // If a refresh token is not included in the response, we can safely assume that the old + // one is still valid and does not need to be discarded + // https://datatracker.ietf.org/doc/html/rfc6749#section-6 var refreshTokenText string var refreshTokenObject *jwt.Token var refreshToken *tokenInfo if result.RefreshToken == nil { - grantType := form.Get(grantTypeField) - if grantType != passwordGrant && grantType != clientCredentialsGrant { - err = fmt.Errorf("no refresh token was received") - return + if w.refreshToken != nil && w.refreshToken.text != "" { + result.RefreshToken = &w.refreshToken.text } } else { refreshTokenText = *result.RefreshToken @@ -1104,7 +1116,6 @@ func parsePullSecretAccessToken(text string) error { const ( grantTypeField = "grant_type" clientIDField = "client_id" - clientSecretField = "client_secret" usernameField = "username" passwordField = "password" refreshTokenField = "refresh_token" diff --git a/authentication/transport_wrapper_test.go b/authentication/transport_wrapper_test.go index d9b7e99b5..8cc605cdb 100644 --- a/authentication/transport_wrapper_test.go +++ b/authentication/transport_wrapper_test.go @@ -1256,6 +1256,57 @@ var _ = Describe("Tokens", func() { Expect(returnedAccess).To(Equal(validAccess)) Expect(returnedRefresh).To(Equal(validRefresh)) }) + + It("Uses client credentials grant with basic authentication and opaque refresh token", func() { + // Generate the tokens: + expiredAccess := MakeTokenString("Bearer", -5*time.Minute) + validAccess := MakeTokenString("Bearer", 5*time.Minute) + opaqueRefresh := "a.bb.ccc.dddd.eeeee" + + // Configure the server so that it returns a expired access token for the + // first request, and then a valid access token for the second request. In + // both cases the client should be using the client credentials grant with + // basic authentication. + server.AppendHandlers( + CombineHandlers( + VerifyClientCredentialsGrant("myclient", "mysecret"), + RespondWithAccessToken(expiredAccess), + ), + CombineHandlers( + VerifyClientCredentialsGrant("myclient", "mysecret"), + RespondWithAccessToken(validAccess), + ), + ) + + // Create the wrapper: + wrapper, err := NewTransportWrapper(). + Logger(logger). + TokenURL(server.URL()). + TrustedCA(ca). + Client("myclient", "mysecret"). + Tokens(expiredAccess, opaqueRefresh). + Build(ctx) + Expect(err).ToNot(HaveOccurred()) + defer func() { + err = wrapper.Close() + Expect(err).ToNot(HaveOccurred()) + }() + + // Force the initial token request. This will return an expired access token + // and a valid refresh token, that way when we get the tokens again the + // wrapper should send another request, but using the client credentials + // grant and ignoring the refresh token. + returnedAccess, returnedRefresh, err := wrapper.Tokens(ctx) + Expect(err).ToNot(HaveOccurred()) + Expect(returnedAccess).To(Equal(expiredAccess)) + Expect(returnedRefresh).To(Equal(opaqueRefresh)) + + // Force another request: + returnedAccess, returnedRefresh, err = wrapper.Tokens(ctx) + Expect(err).ToNot(HaveOccurred()) + Expect(returnedAccess).To(Equal(validAccess)) + Expect(returnedRefresh).To(Equal(opaqueRefresh)) + }) }) Describe("Retry for getting access token", func() { @@ -1445,9 +1496,9 @@ func VerifyClientCredentialsGrant(id, secret string) http.HandlerFunc { return CombineHandlers( VerifyRequest(http.MethodPost, "/"), VerifyContentType("application/x-www-form-urlencoded"), + VerifyBasicAuth(id, secret), VerifyFormKV("grant_type", "client_credentials"), VerifyFormKV("client_id", id), - VerifyFormKV("client_secret", secret), ) }