From ac6e419d480cce2fbf5b5c167f4766b05304ad68 Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Mon, 10 Oct 2016 11:02:27 -0700 Subject: [PATCH] server: add tests for refreshing with explicit scopes --- server/handlers.go | 21 ++-- server/oauth2.go | 1 + server/server_test.go | 258 +++++++++++++++++++++++++++--------------- 3 files changed, 183 insertions(+), 97 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index 40621adc..02c1881b 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -538,20 +538,25 @@ func (s *Server) handleRefreshToken(w http.ResponseWriter, r *http.Request, clie scopes := refresh.Scopes if scope != "" { requestedScopes := strings.Split(scope, " ") - contains := func() bool { - Loop: - for _, s := range requestedScopes { + var unauthorizedScopes []string + + for _, s := range requestedScopes { + contains := func() bool { for _, scope := range refresh.Scopes { if s == scope { - continue Loop + return true } } return false + }() + if !contains { + unauthorizedScopes = append(unauthorizedScopes, s) } - return true - }() - if !contains { - tokenErr(w, errInvalidRequest, "Requested scopes did not contain authorized scopes.", http.StatusBadRequest) + } + + if len(unauthorizedScopes) > 0 { + msg := fmt.Sprintf("Requested scopes contain unauthorized scope(s): %q.", unauthorizedScopes) + tokenErr(w, errInvalidRequest, msg, http.StatusBadRequest) return } scopes = requestedScopes diff --git a/server/oauth2.go b/server/oauth2.go index 003d8947..61f6a923 100644 --- a/server/oauth2.go +++ b/server/oauth2.go @@ -52,6 +52,7 @@ func tokenErr(w http.ResponseWriter, typ, description string, statusCode int) { } w.Header().Set("Content-Type", "application/json") w.Header().Set("Content-Length", strconv.Itoa(len(body))) + w.WriteHeader(statusCode) w.Write(body) } diff --git a/server/server_test.go b/server/server_test.go index ea0793aa..13f46ac1 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -131,111 +131,191 @@ func TestDiscovery(t *testing.T) { } func TestOAuth2CodeFlow(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() + clientID := "testclient" + clientSecret := "testclientsecret" + requestedScopes := []string{oidc.ScopeOpenID, "email", "offline_access"} - httpServer, s := newTestServer(t, func(c *Config) { - c.Issuer = c.Issuer + "/non-root-path" - }) - defer httpServer.Close() - - p, err := oidc.NewProvider(ctx, httpServer.URL) - if err != nil { - t.Fatalf("failed to get provider: %v", err) - } - - var ( - reqDump, respDump []byte - gotCode bool - state = "a_state" - ) - defer func() { - if !gotCode { - t.Errorf("never got a code in callback\n%s\n%s", reqDump, respDump) - } - }() - - var oauth2Config *oauth2.Config - oauth2Server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/callback" { - q := r.URL.Query() - if errType := q.Get("error"); errType != "" { - if desc := q.Get("error_description"); desc != "" { - t.Errorf("got error from server %s: %s", errType, desc) - } else { - t.Errorf("got error from server %s", errType) - } - w.WriteHeader(http.StatusInternalServerError) - return - } - - if code := q.Get("code"); code != "" { - gotCode = true - token, err := oauth2Config.Exchange(ctx, code) - if err != nil { - t.Errorf("failed to exchange code for token: %v", err) - return - } + tests := []struct { + name string + handleToken func(context.Context, *oidc.Provider, *oauth2.Config, *oauth2.Token) error + }{ + { + name: "verify ID Token", + handleToken: func(ctx context.Context, p *oidc.Provider, config *oauth2.Config, token *oauth2.Token) error { idToken, ok := token.Extra("id_token").(string) if !ok { - t.Errorf("no id token found: %v", err) - return + return fmt.Errorf("no id token found") } - // TODO(ericchiang): validate id token - _ = idToken - + if _, err := p.NewVerifier(ctx).Verify(idToken); err != nil { + return fmt.Errorf("failed to verify id token: %v", err) + } + return nil + }, + }, + { + name: "refresh token", + handleToken: func(ctx context.Context, p *oidc.Provider, config *oauth2.Config, token *oauth2.Token) error { + // have to use time.Now because the OAuth2 package uses it. token.Expiry = time.Now().Add(time.Second * -10) if token.Valid() { - t.Errorf("token shouldn't be valid") + return errors.New("token shouldn't be valid") } - newToken, err := oauth2Config.TokenSource(ctx, token).Token() + newToken, err := config.TokenSource(ctx, token).Token() if err != nil { - t.Errorf("failed to refresh token: %v", err) - return + return fmt.Errorf("failed to refresh token: %v", err) } if token.RefreshToken == newToken.RefreshToken { - t.Errorf("old refresh token was the same as the new token %q", token.RefreshToken) + return fmt.Errorf("old refresh token was the same as the new token %q", token.RefreshToken) } + return nil + }, + }, + { + name: "refresh with explicit scopes", + handleToken: func(ctx context.Context, p *oidc.Provider, config *oauth2.Config, token *oauth2.Token) error { + v := url.Values{} + v.Add("client_id", clientID) + v.Add("client_secret", clientSecret) + v.Add("grant_type", "refresh_token") + v.Add("refresh_token", token.RefreshToken) + v.Add("scope", strings.Join(requestedScopes, " ")) + resp, err := http.PostForm(p.TokenURL, v) + if err != nil { + return err + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + dump, err := httputil.DumpResponse(resp, true) + if err != nil { + panic(err) + } + return fmt.Errorf("unexpected response: %s", dump) + } + return nil + }, + }, + { + name: "refresh with unauthorized scopes", + handleToken: func(ctx context.Context, p *oidc.Provider, config *oauth2.Config, token *oauth2.Token) error { + v := url.Values{} + v.Add("client_id", clientID) + v.Add("client_secret", clientSecret) + v.Add("grant_type", "refresh_token") + v.Add("refresh_token", token.RefreshToken) + // Request a scope that wasn't requestd initially. + v.Add("scope", strings.Join(append(requestedScopes, "profile"), " ")) + resp, err := http.PostForm(p.TokenURL, v) + if err != nil { + return err + } + defer resp.Body.Close() + if resp.StatusCode == http.StatusOK { + dump, err := httputil.DumpResponse(resp, true) + if err != nil { + panic(err) + } + return fmt.Errorf("unexpected response: %s", dump) + } + return nil + }, + }, + } + + for _, tc := range tests { + func() { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + httpServer, s := newTestServer(t, func(c *Config) { + c.Issuer = c.Issuer + "/non-root-path" + }) + defer httpServer.Close() + + p, err := oidc.NewProvider(ctx, httpServer.URL) + if err != nil { + t.Fatalf("failed to get provider: %v", err) } - if gotState := q.Get("state"); gotState != state { - t.Errorf("state did not match, want=%q got=%q", state, gotState) + + var ( + reqDump, respDump []byte + gotCode bool + state = "a_state" + ) + defer func() { + if !gotCode { + t.Errorf("never got a code in callback\n%s\n%s", reqDump, respDump) + } + }() + + var oauth2Config *oauth2.Config + oauth2Server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/callback" { + q := r.URL.Query() + if errType := q.Get("error"); errType != "" { + if desc := q.Get("error_description"); desc != "" { + t.Errorf("got error from server %s: %s", errType, desc) + } else { + t.Errorf("got error from server %s", errType) + } + w.WriteHeader(http.StatusInternalServerError) + return + } + + if code := q.Get("code"); code != "" { + gotCode = true + token, err := oauth2Config.Exchange(ctx, code) + if err != nil { + t.Errorf("failed to exchange code for token: %v", err) + return + } + err = tc.handleToken(ctx, p, oauth2Config, token) + if err != nil { + t.Errorf("%s: %v", tc.name, err) + } + return + + } + if gotState := q.Get("state"); gotState != state { + t.Errorf("state did not match, want=%q got=%q", state, gotState) + } + w.WriteHeader(http.StatusOK) + return + } + http.Redirect(w, r, oauth2Config.AuthCodeURL(state), http.StatusSeeOther) + })) + + defer oauth2Server.Close() + + redirectURL := oauth2Server.URL + "/callback" + client := storage.Client{ + ID: clientID, + Secret: clientSecret, + RedirectURIs: []string{redirectURL}, + } + if err := s.storage.CreateClient(client); err != nil { + t.Fatalf("failed to create client: %v", err) } - w.WriteHeader(http.StatusOK) - return - } - http.Redirect(w, r, oauth2Config.AuthCodeURL(state), http.StatusSeeOther) - })) - defer oauth2Server.Close() + oauth2Config = &oauth2.Config{ + ClientID: client.ID, + ClientSecret: client.Secret, + Endpoint: p.Endpoint(), + Scopes: requestedScopes, + RedirectURL: redirectURL, + } - redirectURL := oauth2Server.URL + "/callback" - client := storage.Client{ - ID: "testclient", - Secret: "testclientsecret", - RedirectURIs: []string{redirectURL}, - } - if err := s.storage.CreateClient(client); err != nil { - t.Fatalf("failed to create client: %v", err) - } - - oauth2Config = &oauth2.Config{ - ClientID: client.ID, - ClientSecret: client.Secret, - Endpoint: p.Endpoint(), - Scopes: []string{oidc.ScopeOpenID, "profile", "email", "offline_access"}, - RedirectURL: redirectURL, - } - - resp, err := http.Get(oauth2Server.URL + "/login") - if err != nil { - t.Fatalf("get failed: %v", err) - } - if reqDump, err = httputil.DumpRequest(resp.Request, false); err != nil { - t.Fatal(err) - } - if respDump, err = httputil.DumpResponse(resp, true); err != nil { - t.Fatal(err) + resp, err := http.Get(oauth2Server.URL + "/login") + if err != nil { + t.Fatalf("get failed: %v", err) + } + if reqDump, err = httputil.DumpRequest(resp.Request, false); err != nil { + t.Fatal(err) + } + if respDump, err = httputil.DumpResponse(resp, true); err != nil { + t.Fatal(err) + } + }() } }