Merge pull request #1952 from flant/auth-code-iinvalid-grant

fix: return invalid_grant error for invalid or expired auth codes
This commit is contained in:
Márk Sági-Kazár 2021-02-10 15:50:18 +01:00 committed by GitHub
commit 9b1ecac0d9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 107 additions and 1 deletions

View file

@ -805,13 +805,18 @@ func (s *Server) handleAuthCode(w http.ResponseWriter, r *http.Request, client s
code := r.PostFormValue("code") code := r.PostFormValue("code")
redirectURI := r.PostFormValue("redirect_uri") redirectURI := r.PostFormValue("redirect_uri")
if code == "" {
s.tokenErrHelper(w, errInvalidRequest, `Required param: code.`, http.StatusBadRequest)
return
}
authCode, err := s.storage.GetAuthCode(code) authCode, err := s.storage.GetAuthCode(code)
if err != nil || s.now().After(authCode.Expiry) || authCode.ClientID != client.ID { if err != nil || s.now().After(authCode.Expiry) || authCode.ClientID != client.ID {
if err != storage.ErrNotFound { if err != storage.ErrNotFound {
s.logger.Errorf("failed to get auth code: %v", err) s.logger.Errorf("failed to get auth code: %v", err)
s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError) s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError)
} else { } else {
s.tokenErrHelper(w, errInvalidRequest, "Invalid or expired code parameter.", http.StatusBadRequest) s.tokenErrHelper(w, errInvalidGrant, "Invalid or expired code parameter.", http.StatusBadRequest)
} }
return return
} }

View file

@ -10,7 +10,10 @@ import (
"testing" "testing"
"time" "time"
"github.com/coreos/go-oidc/v3/oidc"
"github.com/gorilla/mux" "github.com/gorilla/mux"
"github.com/stretchr/testify/require"
"golang.org/x/oauth2"
"github.com/dexidp/dex/storage" "github.com/dexidp/dex/storage"
"github.com/dexidp/dex/storage/memory" "github.com/dexidp/dex/storage/memory"
@ -204,3 +207,101 @@ func TestConnectorLoginDoesNotAllowToChangeConnectorForAuthRequest(t *testing.T)
t.Error("attempt to overwrite connector on auth request should fail") t.Error("attempt to overwrite connector on auth request should fail")
} }
} }
// TestHandleAuthCode checks that it is forbidden to use same code twice
func TestHandleAuthCode(t *testing.T) {
tests := []struct {
name string
handleCode func(*testing.T, context.Context, *oauth2.Config, string)
}{
{
name: "Code Reuse should return invalid_grant",
handleCode: func(t *testing.T, ctx context.Context, oauth2Config *oauth2.Config, code string) {
_, err := oauth2Config.Exchange(ctx, code)
require.NoError(t, err)
_, err = oauth2Config.Exchange(ctx, code)
require.Error(t, err)
oauth2Err, ok := err.(*oauth2.RetrieveError)
require.True(t, ok)
var errResponse struct{ Error string }
err = json.Unmarshal(oauth2Err.Body, &errResponse)
require.NoError(t, err)
// invalid_grant must be returned for invalid values
// https://tools.ietf.org/html/rfc6749#section-5.2
require.Equal(t, errInvalidGrant, errResponse.Error)
},
},
{
name: "No Code should return invalid_request",
handleCode: func(t *testing.T, ctx context.Context, oauth2Config *oauth2.Config, _ string) {
_, err := oauth2Config.Exchange(ctx, "")
require.Error(t, err)
oauth2Err, ok := err.(*oauth2.RetrieveError)
require.True(t, ok)
var errResponse struct{ Error string }
err = json.Unmarshal(oauth2Err.Body, &errResponse)
require.NoError(t, err)
require.Equal(t, errInvalidRequest, errResponse.Error)
},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
httpServer, s := newTestServer(ctx, t, func(c *Config) { c.Issuer += "/non-root-path" })
defer httpServer.Close()
p, err := oidc.NewProvider(ctx, httpServer.URL)
require.NoError(t, err)
var oauth2Client oauth2Client
oauth2Client.server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/callback" {
http.Redirect(w, r, oauth2Client.config.AuthCodeURL(""), http.StatusSeeOther)
return
}
q := r.URL.Query()
require.Equal(t, q.Get("error"), "", q.Get("error_description"))
code := q.Get("code")
tc.handleCode(t, ctx, oauth2Client.config, code)
w.WriteHeader(http.StatusOK)
}))
defer oauth2Client.server.Close()
redirectURL := oauth2Client.server.URL + "/callback"
client := storage.Client{
ID: "testclient",
Secret: "testclientsecret",
RedirectURIs: []string{redirectURL},
}
err = s.storage.CreateClient(client)
require.NoError(t, err)
oauth2Client.config = &oauth2.Config{
ClientID: client.ID,
ClientSecret: client.Secret,
Endpoint: p.Endpoint(),
Scopes: []string{oidc.ScopeOpenID, "email", "offline_access"},
RedirectURL: redirectURL,
}
resp, err := http.Get(oauth2Client.server.URL + "/login")
require.NoError(t, err)
resp.Body.Close()
})
}
}