dexidp · ericchiang · Jan 11, 2017 · Dec 22, 2016 · Dec 23, 2016 · Dec 23, 2016
diff --git a/Makefile b/Makefile
@@ -55,7 +55,7 @@ fmt:
 	@go fmt $(shell go list ./... | grep -v '/vendor/')
 
 lint:
-	@for package in $(shell go list ./... | grep -v '/vendor/' | grep -v '/api'); do \
+	@for package in $(shell go list ./... | grep -v '/vendor/' | grep -v '/api' | grep -v '/server/internal'); do \
       golint -set_exit_status $$package $$i || exit 1; \
 	done
 
@@ -81,12 +81,15 @@ aci: clean-release _output/bin/dex _output/images/library-alpine-3.4.aci
 docker-image: clean-release _output/bin/dex
 	@sudo docker build -t $(DOCKER_IMAGE) .
 
-.PHONY: grpc
-grpc: api/api.pb.go
+.PHONY: proto
+proto: api/api.pb.go server/internal/types.pb.go
 
 api/api.pb.go: api/api.proto bin/protoc bin/protoc-gen-go
 	@protoc --go_out=plugins=grpc:. api/*.proto
 
+server/internal/types.pb.go: server/internal/types.proto bin/protoc bin/protoc-gen-go
+	@protoc --go_out=. server/internal/*.proto
+
 bin/protoc: scripts/get-protoc
 	@./scripts/get-protoc bin/protoc
 

diff --git a/cmd/example-app/main.go b/cmd/example-app/main.go
@@ -241,7 +241,7 @@ func (a *app) handleLogin(w http.ResponseWriter, r *http.Request) {
 
 	authCodeURL := ""
 	scopes = append(scopes, "openid", "profile", "email")
-	if r.FormValue("offline_acecss") != "yes" {
+	if r.FormValue("offline_access") != "yes" {
 		authCodeURL = a.oauth2Config(scopes).AuthCodeURL(exampleAppState)
 	} else if a.offlineAsScope {
 		scopes = append(scopes, "offline_access")
@@ -254,34 +254,42 @@ func (a *app) handleLogin(w http.ResponseWriter, r *http.Request) {
 }
 
 func (a *app) handleCallback(w http.ResponseWriter, r *http.Request) {
-	if errMsg := r.FormValue("error"); errMsg != "" {
-		http.Error(w, errMsg+": "+r.FormValue("error_description"), http.StatusBadRequest)
-		return
-	}
-
-	if state := r.FormValue("state"); state != exampleAppState {
-		http.Error(w, fmt.Sprintf("expected state %q got %q", exampleAppState, state), http.StatusBadRequest)
-		return
-	}
-
-	code := r.FormValue("code")
-	refresh := r.FormValue("refresh_token")
 	var (
 		err   error
 		token *oauth2.Token
 	)
 	oauth2Config := a.oauth2Config(nil)
-	switch {
-	case code != "":
+	switch r.Method {
+	case "GET":
+		// Authorization redirect callback from OAuth2 auth flow.
+		if errMsg := r.FormValue("error"); errMsg != "" {
+			http.Error(w, errMsg+": "+r.FormValue("error_description"), http.StatusBadRequest)
+			return
+		}
+		code := r.FormValue("code")
+		if code == "" {
+			http.Error(w, fmt.Sprintf("no code in request: %q", r.Form), http.StatusBadRequest)
+			return
+		}
+		if state := r.FormValue("state"); state != exampleAppState {
+			http.Error(w, fmt.Sprintf("expected state %q got %q", exampleAppState, state), http.StatusBadRequest)
+			return
+		}
 		token, err = oauth2Config.Exchange(a.ctx, code)
-	case refresh != "":
+	case "POST":
+		// Form request from frontend to refresh a token.
+		refresh := r.FormValue("refresh_token")
+		if refresh == "" {
+			http.Error(w, fmt.Sprintf("no refresh_token in request: %q", r.Form), http.StatusBadRequest)
+			return
+		}
 		t := &oauth2.Token{
 			RefreshToken: refresh,
 			Expiry:       time.Now().Add(-time.Hour),
 		}
 		token, err = oauth2Config.TokenSource(r.Context(), t).Token()
 	default:
-		http.Error(w, fmt.Sprintf("no code in request: %q", r.Form), http.StatusBadRequest)
+		http.Error(w, fmt.Sprintf("method not implemented: %s", r.Method), http.StatusBadRequest)
 		return
 	}
 

diff --git a/cmd/example-app/templates.go b/cmd/example-app/templates.go
@@ -8,7 +8,7 @@ import (
 
 var indexTmpl = template.Must(template.New("index.html").Parse(`<html>
   <body>
-    <form action="/login">
+    <form action="/login" method="post">
        <p>
          Authenticate for:<input type="text" name="cross_client" placeholder="list of client-ids">
        </p>
@@ -50,8 +50,13 @@ pre {
   <body>
     <p> Token: <pre><code>{{ .IDToken }}</code></pre></p>
     <p> Claims: <pre><code>{{ .Claims }}</code></pre></p>
+	{{ if .RefreshToken }}
     <p> Refresh Token: <pre><code>{{ .RefreshToken }}</code></pre></p>
-    <p><a href="{{ .RedirectURL }}?refresh_token={{ .RefreshToken }}">Redeem refresh token</a><p>
+	<form action="{{ .RedirectURL }}" method="post">
+	  <input type="hidden" name="refresh_token" value="{{ .RefreshToken }}">
+	  <input type="submit" value="Redeem refresh token">
+    </form>
+	{{ end }}
   </body>
 </html>
 `))

diff --git a/server/handlers.go b/server/handlers.go
@@ -2,6 +2,7 @@ package server
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/url"
@@ -16,6 +17,7 @@ import (
 	jose "gopkg.in/square/go-jose.v2"
 
 	"github.com/coreos/dex/connector"
+	"github.com/coreos/dex/server/internal"
 	"github.com/coreos/dex/storage"
 )
 
@@ -645,20 +647,32 @@ func (s *Server) handleAuthCode(w http.ResponseWriter, r *http.Request, client s
 	var refreshToken string
 	if reqRefresh {
 		refresh := storage.RefreshToken{
-			RefreshToken:  storage.NewID(),
+			ID:            storage.NewID(),
+			Token:         storage.NewID(),
 			ClientID:      authCode.ClientID,
 			ConnectorID:   authCode.ConnectorID,
 			Scopes:        authCode.Scopes,
 			Claims:        authCode.Claims,
 			Nonce:         authCode.Nonce,
 			ConnectorData: authCode.ConnectorData,
+			CreatedAt:     s.now(),
+			LastUsed:      s.now(),
 		}
+		token := &internal.RefreshToken{
+			RefreshId: refresh.ID,
+			Token:     refresh.Token,
+		}
+		if refreshToken, err = internal.Marshal(token); err != nil {
+			s.logger.Errorf("failed to marshal refresh token: %v", err)
+			s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError)
+			return
+		}
+
 		if err := s.storage.CreateRefresh(refresh); err != nil {
 			s.logger.Errorf("failed to create refresh token: %v", err)
 			s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError)
 			return
 		}
-		refreshToken = refresh.RefreshToken
 	}
 	s.writeAccessToken(w, idToken, refreshToken, expiry)
 }
@@ -672,16 +686,37 @@ func (s *Server) handleRefreshToken(w http.ResponseWriter, r *http.Request, clie
 		return
 	}
 
-	refresh, err := s.storage.GetRefresh(code)
-	if err != nil || refresh.ClientID != client.ID {
-		if err != storage.ErrNotFound {
-			s.logger.Errorf("failed to get auth code: %v", err)
-			s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError)
-		} else {
+	token := new(internal.RefreshToken)
+	if err := internal.Unmarshal(code, token); err != nil {
+		// For backward compatibility, assume the refresh_token is a raw refresh token ID
+		// if it fails to decode.
+		//
+		// Because refresh_token values that aren't unmarshable were generated by servers
+		// that don't have a Token value, we'll still reject any attempts to claim a
+		// refresh_token twice.
+		token = &internal.RefreshToken{RefreshId: code, Token: ""}
+	}
+
+	refresh, err := s.storage.GetRefresh(token.RefreshId)
+	if err != nil {
+		s.logger.Errorf("failed to get refresh token: %v", err)
+		if err == storage.ErrNotFound {
 			s.tokenErrHelper(w, errInvalidRequest, "Refresh token is invalid or has already been claimed by another client.", http.StatusBadRequest)
+		} else {
+			s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError)
 		}
 		return
 	}
+	if refresh.ClientID != client.ID {
+		s.logger.Errorf("client %s trying to claim token for client %s", client.ID, refresh.ClientID)
+		s.tokenErrHelper(w, errInvalidRequest, "Refresh token is invalid or has already been claimed by another client.", http.StatusBadRequest)
+		return
+	}
+	if refresh.Token != token.Token {
+		s.logger.Errorf("refresh token with id %s claimed twice", refresh.ID)
+		s.tokenErrHelper(w, errInvalidRequest, "Refresh token is invalid or has already been claimed by another client.", http.StatusBadRequest)
+		return
+	}
 
 	// Per the OAuth2 spec, if the client has omitted the scopes, default to the original
 	// authorized scopes.
@@ -720,59 +755,78 @@ func (s *Server) handleRefreshToken(w http.ResponseWriter, r *http.Request, clie
 		s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError)
 		return
 	}
+	ident := connector.Identity{
+		UserID:        refresh.Claims.UserID,
+		Username:      refresh.Claims.Username,
+		Email:         refresh.Claims.Email,
+		EmailVerified: refresh.Claims.EmailVerified,
+		Groups:        refresh.Claims.Groups,
+		ConnectorData: refresh.ConnectorData,
+	}
 
 	// Can the connector refresh the identity? If so, attempt to refresh the data
 	// in the connector.
 	//
 	// TODO(ericchiang): We may want a strict mode where connectors that don't implement
 	// this interface can't perform refreshing.
 	if refreshConn, ok := conn.Connector.(connector.RefreshConnector); ok {
-		ident := connector.Identity{
-			UserID:        refresh.Claims.UserID,
-			Username:      refresh.Claims.Username,
-			Email:         refresh.Claims.Email,
-			EmailVerified: refresh.Claims.EmailVerified,
-			Groups:        refresh.Claims.Groups,
-			ConnectorData: refresh.ConnectorData,
-		}
-		ident, err := refreshConn.Refresh(r.Context(), parseScopes(scopes), ident)
+		newIdent, err := refreshConn.Refresh(r.Context(), parseScopes(scopes), ident)
 		if err != nil {
 			s.logger.Errorf("failed to refresh identity: %v", err)
 			s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError)
 			return
 		}
+		ident = newIdent
+	}
 
-		// Update the claims of the refresh token.
-		//
-		// UserID intentionally ignored for now.
-		refresh.Claims.Username = ident.Username
-		refresh.Claims.Email = ident.Email
-		refresh.Claims.EmailVerified = ident.EmailVerified
-		refresh.Claims.Groups = ident.Groups
-		refresh.ConnectorData = ident.ConnectorData
+	claims := storage.Claims{
+		UserID:        ident.UserID,
+		Username:      ident.Username,
+		Email:         ident.Email,
+		EmailVerified: ident.EmailVerified,
+		Groups:        ident.Groups,
 	}
 
-	idToken, expiry, err := s.newIDToken(client.ID, refresh.Claims, scopes, refresh.Nonce)
+	idToken, expiry, err := s.newIDToken(client.ID, claims, scopes, refresh.Nonce)
 	if err != nil {
 		s.logger.Errorf("failed to create ID token: %v", err)
 		s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError)
 		return
 	}
 
-	// Refresh tokens are claimed exactly once. Delete the current token and
-	// create a new one.
-	if err := s.storage.DeleteRefresh(code); err != nil {
-		s.logger.Errorf("failed to delete auth code: %v", err)
+	newToken := &internal.RefreshToken{
+		RefreshId: refresh.ID,
+		Token:     storage.NewID(),
+	}
+	rawNewToken, err := internal.Marshal(newToken)
+	if err != nil {
+		s.logger.Errorf("failed to marshal refresh token: %v", err)
 		s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError)
 		return
 	}
-	refresh.RefreshToken = storage.NewID()
-	if err := s.storage.CreateRefresh(refresh); err != nil {
-		s.logger.Errorf("failed to create refresh token: %v", err)
+
+	updater := func(old storage.RefreshToken) (storage.RefreshToken, error) {
+		if old.Token != refresh.Token {
+			return old, errors.New("refresh token claimed twice")
+		}
+		old.Token = newToken.Token
+		// Update the claims of the refresh token.
+		//
+		// UserID intentionally ignored for now.
+		old.Claims.Username = ident.Username
+		old.Claims.Email = ident.Email
+		old.Claims.EmailVerified = ident.EmailVerified
+		old.Claims.Groups = ident.Groups
+		old.ConnectorData = ident.ConnectorData
+		old.LastUsed = s.now()
+		return old, nil
+	}
+	if err := s.storage.UpdateRefreshToken(refresh.ID, updater); err != nil {
+		s.logger.Errorf("failed to update refresh token: %v", err)
 		s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError)
 		return
 	}
-	s.writeAccessToken(w, idToken, refresh.RefreshToken, expiry)
+	s.writeAccessToken(w, idToken, rawNewToken, expiry)
 }
 
 func (s *Server) writeAccessToken(w http.ResponseWriter, idToken, refreshToken string, expiry time.Time) {

diff --git a/server/internal/codec.go b/server/internal/codec.go
@@ -0,0 +1,25 @@
+package internal
+
+import (
+	"encoding/base64"
+
+	"github.com/golang/protobuf/proto"
+)
+
+// Marshal converts a protobuf message to a URL legal string.
+func Marshal(message proto.Message) (string, error) {
+	data, err := proto.Marshal(message)
+	if err != nil {
+		return "", err
+	}
+	return base64.RawURLEncoding.EncodeToString(data), nil
+}
+
+// Unmarshal decodes a protobuf message.
+func Unmarshal(s string, message proto.Message) error {
+	data, err := base64.RawURLEncoding.DecodeString(s)
+	if err != nil {
+		return err
+	}
+	return proto.Unmarshal(data, message)
+}