From 9502a332654f89d8722ec7025026f282aea41b66 Mon Sep 17 00:00:00 2001 From: Stephan Renatus Date: Mon, 13 Nov 2017 09:56:18 +0100 Subject: [PATCH 1/5] handlers/oauth2: take id_token_hint Signed-off-by: Stephan Renatus --- server/handlers.go | 6 ++++++ server/internal/codec.go | 10 ++++++++++ server/oauth2.go | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+) diff --git a/server/handlers.go b/server/handlers.go index 926b400009..a8f22ad145 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -167,6 +167,12 @@ func (s *Server) handleAuthorization(w http.ResponseWriter, r *http.Request) { return } + // if we already know which connector the user should use -- go there directly + if authReq.ConnectorID != "" { + http.Redirect(w, r, s.absPath("/auth", authReq.ConnectorID)+"?req="+authReq.ID, http.StatusFound) + return + } + connectors, e := s.storage.ListConnectors() if e != nil { s.logger.Errorf("Failed to get list of connectors: %v", err) diff --git a/server/internal/codec.go b/server/internal/codec.go index a92c26f922..0e7c9bd615 100644 --- a/server/internal/codec.go +++ b/server/internal/codec.go @@ -2,6 +2,7 @@ package internal import ( "encoding/base64" + "encoding/json" "github.com/golang/protobuf/proto" ) @@ -23,3 +24,12 @@ func Unmarshal(s string, message proto.Message) error { } return proto.Unmarshal(data, message) } + +// UnmarshalJSON unmarshals the subject claim's internal format +func (s *IDTokenSubject) UnmarshalJSON(src []byte) error { + var sub string + if err := json.Unmarshal(src, &sub); err != nil { + return err + } + return Unmarshal(sub, s) +} diff --git a/server/oauth2.go b/server/oauth2.go index 6a0d5eee55..38beaa0045 100644 --- a/server/oauth2.go +++ b/server/oauth2.go @@ -376,6 +376,15 @@ func (s *Server) parseAuthorizationRequest(r *http.Request) (req storage.AuthReq scopes := strings.Fields(q.Get("scope")) responseTypes := strings.Fields(q.Get("response_type")) + var connectorID string + if hint := q.Get("id_token_hint"); hint != "" { + connectorID, err = connectorIDFromIDTokenHint(hint) + if err != nil { + s.logger.Errorf("failed to process id_token_hint: %s", err) + return req, &authErr{"", "", errInvalidRequest, "Invalid id_token_hint."} + } + } + client, err := s.storage.GetClient(clientID) if err != nil { if err == storage.ErrNotFound { @@ -484,6 +493,7 @@ func (s *Server) parseAuthorizationRequest(r *http.Request) (req storage.AuthReq return storage.AuthRequest{ ID: storage.NewID(), ClientID: client.ID, + ConnectorID: connectorID, State: state, Nonce: nonce, ForceApprovalPrompt: q.Get("approval_prompt") == "force", @@ -548,3 +558,27 @@ func validateRedirectURI(client storage.Client, redirectURI string) bool { host, _, err := net.SplitHostPort(u.Host) return err == nil && host == "localhost" } + +// connectorIDFromIDTokenHint tries to extract the connector used when the +// passed ID token was issued. It does NOT check the JWT signature -- we might +// get presented an ID token that has long since expired and we still want to +// provide the correct connector pre-selection. Note, however, that the token +// MAY BE VALID and should this not be part of any error output. +func connectorIDFromIDTokenHint(hint string) (string, error) { + parts := strings.SplitN(hint, ".", 3) + if len(parts) != 3 { + return "", errors.New("wrong format") + } + payload, err := base64.RawURLEncoding.DecodeString(parts[1]) + if err != nil { + return "", fmt.Errorf("failed to decode payload: %v", err) + } + + cl := struct { + Sub internal.IDTokenSubject `json:"sub"` + }{} + if err := json.Unmarshal([]byte(payload), &cl); err != nil { + return "", fmt.Errorf("failed to unmarshal payload: %v", err) + } + return cl.Sub.ConnId, nil +} From 326064ff89e59b3fe94664de2aeedc4bd90e1284 Mon Sep 17 00:00:00 2001 From: Stephan Renatus Date: Mon, 13 Nov 2017 11:00:10 +0100 Subject: [PATCH 2/5] example-app: add id_token_hint "support" Signed-off-by: Stephan Renatus --- cmd/example-app/main.go | 15 ++++++++++++--- cmd/example-app/templates.go | 14 +++++++++----- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/cmd/example-app/main.go b/cmd/example-app/main.go index d527bbc8e8..8a0ee5005a 100644 --- a/cmd/example-app/main.go +++ b/cmd/example-app/main.go @@ -278,12 +278,21 @@ func (a *app) handleCallback(w http.ResponseWriter, r *http.Request) { } token, err = oauth2Config.Exchange(ctx, code) case "POST": - // Form request from frontend to refresh a token. + // Form request from frontend to refresh a token; or login again with hint refresh := r.FormValue("refresh_token") - if refresh == "" { - http.Error(w, fmt.Sprintf("no refresh_token in request: %q", r.Form), http.StatusBadRequest) + idTokenHint := r.FormValue("id_token_hint") + if refresh == "" && idTokenHint == "" { + http.Error(w, fmt.Sprintf("no refresh_token or id_token_hint in request: %q", r.Form), http.StatusBadRequest) return } + if idTokenHint != "" { + // redirect to auth URL with the hint, using default scopes + scopes := []string{"openid", "profile", "email"} + authURL := a.oauth2Config(scopes).AuthCodeURL(exampleAppState) + http.Redirect(w, r, authURL+"&id_token_hint="+idTokenHint, http.StatusSeeOther) + return + } + // reaching this means refresh_token handling t := &oauth2.Token{ RefreshToken: refresh, Expiry: time.Now().Add(-time.Hour), diff --git a/cmd/example-app/templates.go b/cmd/example-app/templates.go index a870d0f0a9..09e47e98b9 100644 --- a/cmd/example-app/templates.go +++ b/cmd/example-app/templates.go @@ -50,13 +50,17 @@ pre {

Token: 

{{ .IDToken }}

Claims: 

{{ .Claims }}

- {{ if .RefreshToken }} + {{ if .RefreshToken }}

Refresh Token: 

{{ .RefreshToken }}

-
- - + + + +
+ {{ end }} +
+ +
- {{ end }} `)) From b01faa1a2c15258519fa022b108911c844ea6082 Mon Sep 17 00:00:00 2001 From: Stephan Renatus Date: Tue, 14 Nov 2017 11:35:27 +0100 Subject: [PATCH 3/5] oauth2_test: use t.Run() This way, we automatically get nicer output that has the test case name prefixed: --- PASS: TestParseAuthorizationRequest (0.01s) --- PASS: TestParseAuthorizationRequest/normal_request (0.00s) --- PASS: TestParseAuthorizationRequest/POST_request (0.00s) --- PASS: TestParseAuthorizationRequest/invalid_client_id (0.00s) --- PASS: TestParseAuthorizationRequest/invalid_redirect_uri (0.00s) --- PASS: TestParseAuthorizationRequest/implicit_flow (0.00s) --- PASS: TestParseAuthorizationRequest/unsupported_response_type (0.00s) --- PASS: TestParseAuthorizationRequest/only_token_response_type (0.00s) Signed-off-by: Stephan Renatus --- server/oauth2_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/oauth2_test.go b/server/oauth2_test.go index dcf4947b33..7bb6d0fa0d 100644 --- a/server/oauth2_test.go +++ b/server/oauth2_test.go @@ -145,7 +145,7 @@ func TestParseAuthorizationRequest(t *testing.T) { } for _, tc := range tests { - func() { + t.Run(tc.name, func(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -170,12 +170,12 @@ func TestParseAuthorizationRequest(t *testing.T) { } _, err := server.parseAuthorizationRequest(req) if err != nil && !tc.wantErr { - t.Errorf("%s: %v", tc.name, err) + t.Error(err) } if err == nil && tc.wantErr { - t.Errorf("%s: expected error", tc.name) + t.Error("expected error") } - }() + }) } } From 1e82ba528f455888bc09eb6d8eb955492b927192 Mon Sep 17 00:00:00 2001 From: Stephan Renatus Date: Tue, 14 Nov 2017 11:56:38 +0100 Subject: [PATCH 4/5] oauth2_test: add tests for id_token_hint functionality Signed-off-by: Stephan Renatus --- server/oauth2_test.go | 69 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 67 insertions(+), 2 deletions(-) diff --git a/server/oauth2_test.go b/server/oauth2_test.go index 7bb6d0fa0d..3df9de6e93 100644 --- a/server/oauth2_test.go +++ b/server/oauth2_test.go @@ -24,6 +24,8 @@ func TestParseAuthorizationRequest(t *testing.T) { queryParams map[string]string wantErr bool + + authReqCheck func(*testing.T, *storage.AuthRequest) }{ { name: "normal request", @@ -41,6 +43,66 @@ func TestParseAuthorizationRequest(t *testing.T) { "scope": "openid email profile", }, }, + { + name: "request with valid id_token_hint", + clients: []storage.Client{ + { + ID: "foo", + RedirectURIs: []string{"https://example.com/foo"}, + }, + }, + supportedResponseTypes: []string{"code"}, + queryParams: map[string]string{ + // sub: "Cg0wLTM4NS0yODA4OS0wEgRtb2Nr" = {0-385-28089-0, "mock"} + "id_token_hint": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJDZzB3TFRNNE5TMHlPREE0T1Mwd0VnUnRiMk5yIn0.M1mYqRIYeAaLeo3B7DWj_Nxm589tbworSGffCIgBz04", + "client_id": "foo", + "redirect_uri": "https://example.com/foo", + "response_type": "code", + "scope": "openid email profile", + }, + authReqCheck: func(t *testing.T, ar *storage.AuthRequest) { + if ar.ConnectorID != "mock" { + t.Errorf("expected connectorID \"mock\", got %v", ar.ConnectorID) + } + }, + }, + { + name: "request with non-jwt id_token_hint", + clients: []storage.Client{ + { + ID: "foo", + RedirectURIs: []string{"https://example.com/foo"}, + }, + }, + supportedResponseTypes: []string{"code"}, + queryParams: map[string]string{ + "id_token_hint": "notevenajwt", + "client_id": "foo", + "redirect_uri": "https://example.com/foo", + "response_type": "code", + "scope": "openid email profile", + }, + wantErr: true, + }, + { + name: "request with invalid id_token_hint (bad sub)", + clients: []storage.Client{ + { + ID: "foo", + RedirectURIs: []string{"https://example.com/foo"}, + }, + }, + supportedResponseTypes: []string{"code"}, + queryParams: map[string]string{ + // sub: "ject" + "id_token_hint": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJqZWN0In0.GwGd9UBSu2XrfeULp8u3KI0jZPt1ccUIGk1TaCCtLqE", + "client_id": "foo", + "redirect_uri": "https://example.com/foo", + "response_type": "code", + "scope": "openid email profile", + }, + wantErr: true, + }, { name: "POST request", clients: []storage.Client{ @@ -168,13 +230,16 @@ func TestParseAuthorizationRequest(t *testing.T) { } else { req = httptest.NewRequest("GET", httpServer.URL+"/auth?"+params.Encode(), nil) } - _, err := server.parseAuthorizationRequest(req) + resp, err := server.parseAuthorizationRequest(req) if err != nil && !tc.wantErr { - t.Error(err) + t.Fatal(err) } if err == nil && tc.wantErr { t.Error("expected error") } + if tc.authReqCheck != nil { + tc.authReqCheck(t, &resp) + } }) } } From e8fe811dfd963034886d181f9c71fff52aa36b42 Mon Sep 17 00:00:00 2001 From: Stephan Renatus Date: Mon, 4 Dec 2017 16:06:33 +0100 Subject: [PATCH 5/5] handlers/auth: don't display backlink when passing backlink=none Signed-off-by: Stephan Renatus --- server/handlers.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index a8f22ad145..d11d12ef4a 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -169,7 +169,7 @@ func (s *Server) handleAuthorization(w http.ResponseWriter, r *http.Request) { // if we already know which connector the user should use -- go there directly if authReq.ConnectorID != "" { - http.Redirect(w, r, s.absPath("/auth", authReq.ConnectorID)+"?req="+authReq.ID, http.StatusFound) + http.Redirect(w, r, s.absPath("/auth", authReq.ConnectorID)+"?backlink=none&req="+authReq.ID, http.StatusFound) return } @@ -229,7 +229,12 @@ func (s *Server) handleConnectorLogin(w http.ResponseWriter, r *http.Request) { return } scopes := parseScopes(authReq.Scopes) - showBacklink := len(s.connectors) > 1 + + // allows for overriding the backlink display -- this is set when the initial + // request provided an `id_token_hint`, and we've forwarded the client based + // on this token's connector id. + backlinkOverride := r.FormValue("backlink") + showBacklink := len(s.connectors) > 1 && backlinkOverride != "none" switch r.Method { case "GET":