k-space/dex
Archived
10
0
Fork 0
Code Issues Pull Requests Packages Projects Activity

server: fixes for the implicit and hybrid flow

Browse Source 
Accept the following response_type for the implicit flow:

    id_token
    token id_token

And the following for hybrid flow

    code id_token
    code token
    code token id_token

This corrects the previous behavior of the implicit flow, which
only accepted "token" (now correctly rejected).
This commit is contained in:
Eric Chiang 2017-01-09 10:46:16 -08:00
parent ec9d1607b2
commit f926d74157
6 changed files with 318 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

95
server/handlers.go
View File

@ -144,12 +144,23 @@ func (s *Server) discoveryHandler() (http.Handler, error) {
// handleAuthorization handles the OAuth2 auth endpoint.
func (s *Server) handleAuthorization(w http.ResponseWriter, r *http.Request) {
authReq, err := s.parseAuthorizationRequest(s.supportedResponseTypes, r)
authReq, err := s.parseAuthorizationRequest(r)
if err != nil {
s.logger.Errorf("Failed to parse authorization request: %v", err)
s.renderError(w, http.StatusInternalServerError, "Failed to connect to the database.")
if handler, ok := err.Handle(); ok {
// client_id and redirect_uri checked out and we can redirect back to
// the client with the error.
handler.ServeHTTP(w, r)
return
}
// Otherwise render the error to the user.
//
// TODO(ericchiang): Should we just always render the error?
s.renderError(w, err.Status(), err.Error())
return
}
authReq.Expiry = s.now().Add(time.Minute * 30)
if err := s.storage.CreateAuthRequest(authReq); err != nil {
s.logger.Errorf("Failed to create authorization request: %v", err)
@ -397,12 +408,25 @@ func (s *Server) sendCodeResponse(w http.ResponseWriter, r *http.Request, authRe
s.renderError(w, http.StatusInternalServerError, "Invalid redirect URI.")
return
}
q := u.Query()
var (
// Was the initial request using the implicit or hybrid flow instead of
// the "normal" code flow?
implicitOrHybrid = false
// Only present in hybrid or code flow. code.ID == "" if this is not set.
code storage.AuthCode
// ID token returned immediately if the response_type includes "id_token".
// Only valid for implicit and hybrid flows.
idToken string
idTokenExpiry time.Time
)
for _, responseType := range authReq.ResponseTypes {
switch responseType {
case responseTypeCode:
code := storage.AuthCode{
code = storage.AuthCode{
ID: storage.NewID(),
ClientID: authReq.ClientID,
ConnectorID: authReq.ConnectorID,
@ -419,32 +443,73 @@ func (s *Server) sendCodeResponse(w http.ResponseWriter, r *http.Request, authRe
return
}
// Implicit and hybrid flows that try to use the OOB redirect URI are
// rejected earlier. If we got here we're using the code flow.
if authReq.RedirectURI == redirectURIOOB {
if err := s.templates.oob(w, code.ID); err != nil {
s.logger.Errorf("Server template error: %v", err)
}
return
}
q.Set("code", code.ID)
case responseTypeToken:
idToken, expiry, err := s.newIDToken(authReq.ClientID, authReq.Claims, authReq.Scopes, authReq.Nonce)
implicitOrHybrid = true
case responseTypeIDToken:
implicitOrHybrid = true
var err error
idToken, idTokenExpiry, err = s.newIDToken(authReq.ClientID, authReq.Claims, authReq.Scopes, authReq.Nonce)
if err != nil {
s.logger.Errorf("failed to create ID token: %v", err)
s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError)
return
}
v := url.Values{}
v.Set("access_token", storage.NewID())
v.Set("token_type", "bearer")
v.Set("id_token", idToken)
v.Set("state", authReq.State)
v.Set("expires_in", strconv.Itoa(int(expiry.Sub(s.now()).Seconds())))
u.Fragment = v.Encode()
}
}
q.Set("state", authReq.State)
u.RawQuery = q.Encode()
if implicitOrHybrid {
v := url.Values{}
v.Set("access_token", storage.NewID())
v.Set("token_type", "bearer")
v.Set("state", authReq.State)
if idToken != "" {
v.Set("id_token", idToken)
// The hybrid flow with only "code token" or "code id_token" doesn't return an
// "expires_in" value. If "code" wasn't provided, indicating the implicit flow,
// don't add it.
//
// https://openid.net/specs/openid-connect-core-1_0.html#HybridAuthResponse
if code.ID == "" {
v.Set("expires_in", strconv.Itoa(int(idTokenExpiry.Sub(s.now()).Seconds())))
}
}
if code.ID != "" {
v.Set("code", code.ID)
}
// Implicit and hybrid flows return their values as part of the fragment.
//
// HTTP/1.1 303 See Other
// Location: https://client.example.org/cb#
// access_token=SlAV32hkKG
// &token_type=bearer
// &id_token=eyJ0 ... NiJ9.eyJ1c ... I6IjIifX0.DeWt4Qu ... ZXso
// &expires_in=3600
// &state=af0ifjsldkj
//
u.Fragment = v.Encode()
} else {
// The code flow add values to the URL query.
//
// HTTP/1.1 303 See Other
// Location: https://client.example.org/cb?
// code=SplxlOBeZQQYbYS6WxSbIA
// &state=af0ifjsldkj
//
q := u.Query()
q.Set("code", code.ID)
q.Set("state", authReq.State)
u.RawQuery = q.Encode()
}
http.Redirect(w, r, u.String(), http.StatusSeeOther)
}

129
server/oauth2.go
View File

@ -24,20 +24,39 @@ type authErr struct {
Description string
}
func (err *authErr) ServeHTTP(w http.ResponseWriter, r *http.Request) {
v := url.Values{}
v.Add("state", err.State)
v.Add("error", err.Type)
if err.Description != "" {
v.Add("error_description", err.Description)
func (err *authErr) Status() int {
if err.State == errServerError {
return http.StatusInternalServerError
}
var redirectURI string
if strings.Contains(err.RedirectURI, "?") {
redirectURI = err.RedirectURI + "&" + v.Encode()
} else {
redirectURI = err.RedirectURI + "?" + v.Encode()
return http.StatusBadRequest
}
func (err *authErr) Error() string {
return err.Description
}
func (err *authErr) Handle() (http.Handler, bool) {
// Didn't get a valid redirect URI.
if err.RedirectURI == "" {
return nil, false
}
http.Redirect(w, r, redirectURI, http.StatusSeeOther)
hf := func(w http.ResponseWriter, r *http.Request) {
v := url.Values{}
v.Add("state", err.State)
v.Add("error", err.Type)
if err.Description != "" {
v.Add("error_description", err.Description)
}
var redirectURI string
if strings.Contains(err.RedirectURI, "?") {
redirectURI = err.RedirectURI + "&" + v.Encode()
} else {
redirectURI = err.RedirectURI + "?" + v.Encode()
}
http.Redirect(w, r, redirectURI, http.StatusSeeOther)
}
return http.HandlerFunc(hf), true
}
func tokenErr(w http.ResponseWriter, typ, description string, statusCode int) error {
@ -192,20 +211,19 @@ func (s *Server) newIDToken(clientID string, claims storage.Claims, scopes []str
}
// parse the initial request from the OAuth2 client.
//
// For correctness the logic is largely copied from https://github.com/RangelReale/osin.
func (s *Server) parseAuthorizationRequest(supportedResponseTypes map[string]bool, r *http.Request) (req storage.AuthRequest, oauth2Err *authErr) {
if err := r.ParseForm(); err != nil {
return req, &authErr{"", "", errInvalidRequest, "Failed to parse request."}
}
redirectURI, err := url.QueryUnescape(r.Form.Get("redirect_uri"))
func (s *Server) parseAuthorizationRequest(r *http.Request) (req storage.AuthRequest, oauth2Err *authErr) {
q := r.URL.Query()
redirectURI, err := url.QueryUnescape(q.Get("redirect_uri"))
if err != nil {
return req, &authErr{"", "", errInvalidRequest, "No redirect_uri provided."}
}
state := r.FormValue("state")
clientID := r.Form.Get("client_id")
clientID := q.Get("client_id")
state := q.Get("state")
nonce := q.Get("nonce")
// Some clients, like the old go-oidc, provide extra whitespace. Tolerate this.
scopes := strings.Fields(q.Get("scope"))
responseTypes := strings.Fields(q.Get("response_type"))
client, err := s.storage.GetClient(clientID)
if err != nil {
@ -222,12 +240,11 @@ func (s *Server) parseAuthorizationRequest(supportedResponseTypes map[string]boo
return req, &authErr{"", "", errInvalidRequest, description}
}
// From here on out, we want to redirect back to the client with an error.
newErr := func(typ, format string, a ...interface{}) *authErr {
return &authErr{state, redirectURI, typ, fmt.Sprintf(format, a...)}
}
scopes := strings.Fields(r.Form.Get("scope"))
var (
unrecognized []string
invalidScopes []string
@ -247,7 +264,7 @@ func (s *Server) parseAuthorizationRequest(supportedResponseTypes map[string]boo
isTrusted, err := s.validateCrossClientTrust(clientID, peerID)
if err != nil {
return req, newErr(errServerError, "")
return req, newErr(errServerError, "Internal server error.")
}
if !isTrusted {
invalidScopes = append(invalidScopes, scope)
@ -264,37 +281,61 @@ func (s *Server) parseAuthorizationRequest(supportedResponseTypes map[string]boo
return req, newErr("invalid_scope", "Client can't request scope(s) %q", invalidScopes)
}
nonce := r.Form.Get("nonce")
responseTypes := strings.Split(r.Form.Get("response_type"), " ")
var rt struct {
code bool
idToken bool
token bool
}
for _, responseType := range responseTypes {
if !supportedResponseTypes[responseType] {
switch responseType {
case responseTypeCode:
rt.code = true
case responseTypeIDToken:
rt.idToken = true
case responseTypeToken:
rt.token = true
default:
return req, newErr("invalid_request", "Invalid response type %q", responseType)
}
switch responseType {
case responseTypeCode:
case responseTypeToken:
// Implicit flow requires a nonce value.
// https://openid.net/specs/openid-connect-core-1_0.html#ImplicitAuthRequest
if nonce == "" {
return req, newErr("invalid_request", "Response type 'token' requires a 'nonce' value.")
}
if !s.supportedResponseTypes[responseType] {
return req, newErr(errUnsupportedResponseType, "Unsupported response type %q", responseType)
}
}
if redirectURI == redirectURIOOB {
err := fmt.Sprintf("Cannot use response type 'token' with redirect_uri '%s'.", redirectURIOOB)
return req, newErr("invalid_request", err)
}
default:
return req, newErr("invalid_request", "Invalid response type %q", responseType)
if len(responseTypes) == 0 {
return req, newErr("invalid_requests", "No response_type provided")
}
if rt.token && !rt.code && !rt.idToken {
// "token" can't be provided by its own.
//
// https://openid.net/specs/openid-connect-core-1_0.html#Authentication
return req, newErr("invalid_request", "Response type 'token' must be provided with type 'id_token' and/or 'code'")
}
if !rt.code {
// Either "id_token code" or "id_token" has been provided which implies the
// implicit flow. Implicit flow requires a nonce value.
//
// https://openid.net/specs/openid-connect-core-1_0.html#ImplicitAuthRequest
if nonce == "" {
return req, newErr("invalid_request", "Response type 'token' requires a 'nonce' value.")
}
}
if rt.token {
if redirectURI == redirectURIOOB {
err := fmt.Sprintf("Cannot use response type 'token' with redirect_uri '%s'.", redirectURIOOB)
return req, newErr("invalid_request", err)
}
}
return storage.AuthRequest{
ID: storage.NewID(),
ClientID: client.ID,
State: r.Form.Get("state"),
State: state,
Nonce: nonce,
ForceApprovalPrompt: r.Form.Get("approval_prompt") == "force",
ForceApprovalPrompt: q.Get("approval_prompt") == "force",
Scopes: scopes,
RedirectURI: redirectURI,
ResponseTypes: responseTypes,

149
server/oauth2_test.go
View File

@ -1 +1,150 @@
package server
import (
"context"
"net/http/httptest"
"net/url"
"testing"
"github.com/coreos/dex/storage"
)
func TestParseAuthorizationRequest(t *testing.T) {
tests := []struct {
name string
clients []storage.Client
supportedResponseTypes []string
queryParams map[string]string
wantErr bool
}{
{
name: "normal request",
clients: []storage.Client{
{
ID: "foo",
RedirectURIs: []string{"https://example.com/foo"},
},
},
supportedResponseTypes: []string{"code"},
queryParams: map[string]string{
"client_id": "foo",
"redirect_uri": "https://example.com/foo",
"response_type": "code",
"scope": "openid email profile",
},
},
{
name: "invalid client id",
clients: []storage.Client{
{
ID: "foo",
RedirectURIs: []string{"https://example.com/foo"},
},
},
supportedResponseTypes: []string{"code"},
queryParams: map[string]string{
"client_id": "bar",
"redirect_uri": "https://example.com/foo",
"response_type": "code",
"scope": "openid email profile",
},
wantErr: true,
},
{
name: "invalid redirect uri",
clients: []storage.Client{
{
ID: "bar",
RedirectURIs: []string{"https://example.com/bar"},
},
},
supportedResponseTypes: []string{"code"},
queryParams: map[string]string{
"client_id": "bar",
"redirect_uri": "https://example.com/foo",
"response_type": "code",
"scope": "openid email profile",
},
wantErr: true,
},
{
name: "implicit flow",
clients: []storage.Client{
{
ID: "bar",
RedirectURIs: []string{"https://example.com/bar"},
},
},
supportedResponseTypes: []string{"code", "id_token", "token"},
queryParams: map[string]string{
"client_id": "bar",
"redirect_uri": "https://example.com/bar",
"response_type": "code id_token",
"scope": "openid email profile",
},
},
{
name: "unsupported response type",
clients: []storage.Client{
{
ID: "bar",
RedirectURIs: []string{"https://example.com/bar"},
},
},
supportedResponseTypes: []string{"code"},
queryParams: map[string]string{
"client_id": "bar",
"redirect_uri": "https://example.com/bar",
"response_type": "code id_token",
"scope": "openid email profile",
},
wantErr: true,
},
{
name: "only token response type",
clients: []storage.Client{
{
ID: "bar",
RedirectURIs: []string{"https://example.com/bar"},
},
},
supportedResponseTypes: []string{"code", "id_token", "token"},
queryParams: map[string]string{
"client_id": "bar",
"redirect_uri": "https://example.com/bar",
"response_type": "token",
"scope": "openid email profile",
},
wantErr: true,
},
}
for _, tc := range tests {
func() {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
httpServer, server := newTestServer(ctx, t, func(c *Config) {
c.SupportedResponseTypes = tc.supportedResponseTypes
c.Storage = storage.WithStaticClients(c.Storage, tc.clients)
})
defer httpServer.Close()
params := url.Values{}
for k, v := range tc.queryParams {
params.Set(k, v)
}
req := httptest.NewRequest("GET", httpServer.URL+"/auth?"+params.Encode(), nil)
_, err := server.parseAuthorizationRequest(req)
if err != nil && !tc.wantErr {
t.Errorf("%s: %v", tc.name, err)
}
if err == nil && tc.wantErr {
t.Errorf("%s: expected error", tc.name)
}
}()
}
}

1
server/rotation.go
View File

@ -130,6 +130,7 @@ func (k keyRotater) rotate() error {
// Remove expired verification keys.
i := 0
for _, key := range keys.VerificationKeys {
if !key.Expiry.After(tNow) {
keys.VerificationKeys[i] = key

2
server/server.go
View File

@ -159,7 +159,7 @@ func newServer(ctx context.Context, c Config, rotationStrategy rotationStrategy)
supported := make(map[string]bool)
for _, respType := range c.SupportedResponseTypes {
switch respType {
case responseTypeCode, responseTypeToken:
case responseTypeCode, responseTypeIDToken, responseTypeToken:
default:
return nil, fmt.Errorf("unsupported response_type %q", respType)
}

4
server/server_test.go
View File

@ -510,7 +510,7 @@ func TestOAuth2ImplicitFlow(t *testing.T) {
httpServer, s := newTestServer(ctx, t, func(c *Config) {
// Enable support for the implicit flow.
c.SupportedResponseTypes = []string{"code", "token"}
c.SupportedResponseTypes = []string{"code", "token", "id_token"}
})
defer httpServer.Close()
@ -553,7 +553,7 @@ func TestOAuth2ImplicitFlow(t *testing.T) {
w.WriteHeader(http.StatusOK)
return
}
u := oauth2Config.AuthCodeURL(state, oauth2.SetAuthURLParam("response_type", "token"), oidc.Nonce(nonce))
u := oauth2Config.AuthCodeURL(state, oauth2.SetAuthURLParam("response_type", "id_token token"), oidc.Nonce(nonce))
http.Redirect(w, r, u, http.StatusSeeOther)
}))