@@ -2,7 +2,6 @@ package server
22
33import (
44 "context"
5- "crypto/hmac"
65 "crypto/sha256"
76 "crypto/subtle"
87 "encoding/base64"
@@ -13,7 +12,6 @@ import (
1312 "maps"
1413 "net/http"
1514 "net/url"
16- "path"
1715 "sort"
1816 "strconv"
1917 "strings"
@@ -887,30 +885,13 @@ func (s *Server) finalizeLogin(ctx context.Context, identity connector.Identity,
887885 userIdentity = & ui
888886 }
889887
890- // an HMAC is used here to ensure that the request ID is unpredictable, ensuring that an attacker who intercepted the original
891- // flow would be unable to poll for the result at the /approval endpoint
892- h := hmac .New (sha256 .New , authReq .HMACKey )
893- h .Write ([]byte (authReq .ID ))
894- mac := h .Sum (nil )
895- hmacParam := base64 .RawURLEncoding .EncodeToString (mac )
896-
897888 // Check if the client requires MFA.
898889 mfaChain , err := s .mfaChainForClient (ctx , authReq .ClientID , authReq .ConnectorID )
899890 if err != nil {
900891 return "" , false , fmt .Errorf ("failed to get MFA chain for client: %v" , err )
901892 }
902893 if len (mfaChain ) > 0 {
903- // Redirect to MFA verification starting with the first authenticator.
904- // Each authenticator redirects to the next one in the chain upon success.
905- // HMAC includes authenticatorID to prevent skipping steps by URL manipulation.
906- h .Reset ()
907- h .Write ([]byte (authReq .ID + "|" + mfaChain [0 ]))
908- v := url.Values {}
909- v .Set ("req" , authReq .ID )
910- v .Set ("hmac" , base64 .RawURLEncoding .EncodeToString (h .Sum (nil )))
911- v .Set ("authenticator" , mfaChain [0 ])
912- returnURL := path .Join (s .issuerURL .Path , "/mfa/verify" ) + "?" + v .Encode ()
913- return returnURL , false , nil
894+ return s .buildMFARedirectURL (authReq , mfaChain [0 ]), false , nil
914895 }
915896
916897 // No MFA required — mark as validated.
@@ -933,8 +914,7 @@ func (s *Server) finalizeLogin(ctx context.Context, identity connector.Identity,
933914 }
934915 }
935916
936- returnURL := path .Join (s .issuerURL .Path , "/approval" ) + "?req=" + authReq .ID + "&hmac=" + hmacParam
937- return returnURL , false , nil
917+ return s .buildApprovalURL (authReq ), false , nil
938918}
939919
940920func (s * Server ) handleApproval (w http.ResponseWriter , r * http.Request ) {
@@ -944,12 +924,6 @@ func (s *Server) handleApproval(w http.ResponseWriter, r *http.Request) {
944924 s .renderError (r , w , http .StatusUnauthorized , "Unauthorized request" )
945925 return
946926 }
947- mac , err := base64 .RawURLEncoding .DecodeString (macEncoded )
948- if err != nil {
949- s .renderError (r , w , http .StatusUnauthorized , "Unauthorized request" )
950- return
951- }
952-
953927 authReq , err := s .storage .GetAuthRequest (ctx , r .FormValue ("req" ))
954928 if err != nil {
955929 if err == storage .ErrNotFound {
@@ -966,7 +940,6 @@ func (s *Server) handleApproval(w http.ResponseWriter, r *http.Request) {
966940 return
967941 }
968942
969- h := hmac .New (sha256 .New , authReq .HMACKey )
970943 if ! authReq .MFAValidated {
971944 // Check if MFA is actually required — if so, redirect to TOTP instead of blocking.
972945 // This handles the case where MFA was enabled after the auth flow started.
@@ -977,30 +950,37 @@ func (s *Server) handleApproval(w http.ResponseWriter, r *http.Request) {
977950 return
978951 }
979952 if len (mfaChain ) > 0 {
980- h .Write ([]byte (authReq .ID + "|" + mfaChain [0 ]))
981- v := url.Values {}
982- v .Set ("req" , authReq .ID )
983- v .Set ("hmac" , base64 .RawURLEncoding .EncodeToString (h .Sum (nil )))
984- v .Set ("authenticator" , mfaChain [0 ])
985- h .Reset ()
986- totpURL := path .Join (s .issuerURL .Path , "/mfa/verify" ) + "?" + v .Encode ()
987- http .Redirect (w , r , totpURL , http .StatusSeeOther )
953+ http .Redirect (w , r , s .buildMFARedirectURL (authReq , mfaChain [0 ]), http .StatusSeeOther )
988954 return
989955 }
990956 // No MFA required but flag not set — allow through (backward compat).
991957 }
992958
993- // build expected hmac with secret key
994- h .Write ([]byte (authReq .ID ))
995- expectedMAC := h .Sum (nil )
996- // constant time comparison
997- if ! hmac .Equal (mac , expectedMAC ) {
959+ if ! verifyHMAC (authReq .HMACKey , macEncoded , authReq .ID , "" ) {
998960 s .renderError (r , w , http .StatusUnauthorized , "Unauthorized request" )
999961 return
1000962 }
1001963
1002964 switch r .Method {
1003965 case http .MethodGet :
966+ // Skip the approval page and issue the code directly if:
967+ // 1. The client didn't force the approval prompt, AND
968+ // 2. Either the server is configured to skip approval globally,
969+ // or the user has already consented to all requested scopes for this client.
970+ // This handles the MFA redirect case: after MFA completion the user lands on
971+ // /approval via GET, and we don't want to show the consent screen again.
972+ if ! authReq .ForceApprovalPrompt {
973+ if s .skipApproval {
974+ s .sendCodeResponse (w , r , authReq )
975+ return
976+ }
977+ ui , err := s .storage .GetUserIdentity (ctx , authReq .Claims .UserID , authReq .ConnectorID )
978+ if err == nil && scopesCoveredByConsent (ui .Consents [authReq .ClientID ], authReq .Scopes ) {
979+ s .sendCodeResponse (w , r , authReq )
980+ return
981+ }
982+ }
983+
1004984 client , err := s .storage .GetClient (ctx , authReq .ClientID )
1005985 if err != nil {
1006986 s .logger .ErrorContext (r .Context (), "Failed to get client" , "client_id" , authReq .ClientID , "err" , err )
0 commit comments