Repository: coreos/dex (go)

Issue:
Authorization endpoint must support POST
In http://openid.net/specs/openid-connect-core-1_0.html#rfc.section.3.1.2.1 it says:

> Authorization Servers MUST support the use of the HTTP GET and POST methods defined in RFC 2616 [RFC2616] at the Authorization Endpoint. Clients MAY use the HTTP GET or POST methods to send the Authorization Request to the Authorization Server. [...] If using the HTTP POST method, the request parameters are serialized using Form Serialization, per Section 13.2.

I tried fiddling with the authorization endpoint using curl. Example:

```
curl -i -k -X POST \
  -d "client_id=cli&response_type=code&scope=openid&state=" \
  https://docker.dev:5554/dex/auth
```

For the above request, I got this response:

```nohighlight
HTTP/1.1 200 OK
Date: Fri, 27 Jan 2017 11:29:16 GMT
Content-Length: 789
Content-Type: text/html; charset=utf-8

<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8">
    <meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1">
    <title>dex</title>
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <link href="https://docker.dev:5554/dex/static/main.css" rel="stylesheet">
    <link href="https://docker.dev:5554/dex/theme/styles.css" rel="stylesheet">
  </head>

  <body class="theme-body">
    <div class="theme-navbar">
      <div class="theme-navbar__logo-wrap">
        <img class="theme-navbar__logo" src="https://docker.dev:5554/dex/theme/logo.png">
      </div>
    </div>

    <div class="dex-container">



<div class="theme-panel">
  <h2 class="theme-heading">Bad Request</h2>
  <p>Invalid client_id ("").</p>
</div>

    </div>
  </body>
</html>
```

The error message shows that dex does not interpret the `client_id` given via form attributes.

The request content type resulting from the above curl command is `application/x-www-form-urlencoded`.

Pull request:
server: support POSTing to authorization endpoint
cc @rithujohn191 @marians

Fixes #791

Probably a cherry pick candidate.

Patch:
diff --git a/Documentation/github-connector.md b/Documentation/github-connector.md
index 223853bf..6491c78e 100644
--- a/Documentation/github-connector.md
+++ b/Documentation/github-connector.md
@@ -15,9 +15,7 @@ The following is an example of a configuration for `examples/config-dev.yaml`:
 ```yaml
 connectors:
 - type: github
-  # Required field for connector id.
   id: github
-  # Required field for connector name.
   name: GitHub
   config:
     # Credentials can be string literals or pulled from the environment.
diff --git a/Documentation/kubernetes.md b/Documentation/kubernetes.md
index 4b3ce709..836001ab 100644
--- a/Documentation/kubernetes.md
+++ b/Documentation/kubernetes.md
@@ -17,7 +17,7 @@ Username and group information can be combined with Kubernetes [authorization pl
 
 ## Configuring the OpenID Connect plugin
 
-Configuring the API server to use the OpenID Connect [authentication plugin][k8s-oidc] requires:
+Configuring the API server to use the OpenID Connect [authentcation plugin][k8s-oidc] requires:
 
 * Deploying an API server with specific flags.
 * Dex is running on HTTPS.
diff --git a/Documentation/ldap-connector.md b/Documentation/ldap-connector.md
index 9dd3e447..97b444ea 100644
--- a/Documentation/ldap-connector.md
+++ b/Documentation/ldap-connector.md
@@ -24,10 +24,7 @@ The following is an example config file that can be used by the LDAP connector t
 ```yaml
 connectors:
 - type: ldap
-  # Required field for connector id.
   id: ldap
-  # Required field for connector name.
-  name: LDAP
   config:
     # Host and optional port of the LDAP server in the form "host:port".
     # If the port is not supplied, it will be guessed based on "insecureNoSSL".
@@ -191,7 +188,6 @@ The following configuration will allow the LDAP connector to search a FreeIPA di
 connectors:
 - type: ldap
   id: ldap
-  name: LDAP
   config:
     # host and port of the LDAP server in form "host:port".
     host: freeipa.example.com:636
diff --git a/Documentation/proposals/token-revocation.md b/Documentation/proposals/token-revocation.md
deleted file mode 100644
index b4a50eb0..00000000
--- a/Documentation/proposals/token-revocation.md
+++ /dev/null
@@ -1,82 +0,0 @@
-# Proposal: design for revoking refresh tokens.
-
-Refresh tokens are issued to the client by the authorization server and are used
-to request a new access token when the current access token becomes invalid or expires.
-It is a common usecase for the end users to revoke client access to their identity.
-This proposal defines the changes needed in Dex v2 to support refresh token revocation.
-
-## Motivation
-
-1. Currently refresh tokens are not associated with the user. Need a new "session object" for this.
-2. Need an API to list refresh tokens based on the UserID.
-3. We need a way for users to login to dex and revoke a client.
-4. Limit the number refresh tokens for each user-client pair to 1.
-
-## Details
-
-Currently in Dex when an end user successfully logs in via a connector and has the OfflineAccess
-scope set to true, a refresh token is created and stored in the backing datastore. There is no
-association between the end user and the refresh token. Hence if we want to support the functionality
-of users being able to revoke refresh tokens, the first step is to have a structure in place that allows
-us retrieve a list of refresh tokens depending on the authenticated user.
-
-```go
-// Reference object for RefreshToken containing only metadata.
-type RefreshTokenRef struct {
-	// ID of the RefreshToken
-	ID string
-	CreatedAt time.Time
-	LastUsed  time.Time
-}
-
-// Session objects pertaining to users with refresh tokens.
-//
-// Will have to handle garbage collection i.e. if no refresh token exists for a user,
-// this object must be cleaned up.
-type OfflineSession struct {
-        // UserID of an end user who has logged in to the server.
-        UserID        string
-        // The ID of the connector used to login the user.
-        ConnID     string
-        // List of pointers to RefreshTokens issued for SessionID
-        Refresh         []*RefreshTokenRef
-}
-
-// Retrieve OfflineSession obj for given userId and connID
-func getOfflineSession (userId string, connID string)
-
-```
-
-### Changes in Dex CodeFlows
-
-1. Client requests a refresh token:
-   Try to retrieve the `OfflineSession` object for the User with the given `UserID + ConnID`.
-   This leads to two possibilities:   
-	* Object exists: This means a Refresh token already exists for the user.
-          Update the existing `OffilineSession` object with the newly received token as follows:
-		* CreateRefresh() will create a new `RefreshToken` obj in the storage.
-		* Update the `Refresh` list with the new `RefreshToken` pointer.
-		* Delete the old refresh token in storage.
-
-	* No object found: This implies that this will be the first refresh token for the user.
- 		* CreateRefresh() will create a new `RefreshToken` obj in the storage.
-		* Create an OfflineSession for the user and add the new `RefreshToken` pointer to
-		  the `Refresh` list.
-                
-2. Refresh token rotation:
-   There will be no change to this codeflow. When the client refreshes a refresh token, the `TokenID`
-   still remains intact and only the `RefreshToken` obj gets updated with a new nonce. We do not need
-   any additional checks in the OfflineSession objects as the `RefreshToken` pointers still remain intact.
-
-3. User revokes a refresh token (New functionality):
-   A user that has been authenticated externally will have the ability to revoke their refresh tokens.
-   Please note that Dex's API does not perform the authentication, this will have to be done by an
-   external app.
-   Steps involved:
-	* Get `OfflineSession` obj with given UserID + ConnID. 
-	* If a refresh token exists in `Refresh`, delete the `RefreshToken` (handle this in storage)
-	  and its pointer value in `Refresh`. Clean up the OfflineSession object.
-	* If there is no refresh token found, handle error case.
-
-NOTE: To avoid race conditions between “requesting a refresh token” and “revoking a refresh token”, use
-locking mechanism when updating an `OfflineSession` object.
diff --git a/Documentation/saml-connector.md b/Documentation/saml-connector.md
index 28800463..d78a6f2b 100644
--- a/Documentation/saml-connector.md
+++ b/Documentation/saml-connector.md
@@ -19,10 +19,7 @@ The connector doesn't support refresh tokens since the SAML 2.0 protocol doesn't
 ```yaml
 connectors:
 - type: samlExperimental # will be changed to "saml" later without support for the "samlExperimental" value
-  # Required field for connector id.
   id: saml
-  # Required field for connector name.
-  name: SAML
   config:
     # Issuer used for validating the SAML response.
     issuer: https://saml.example.com
diff --git a/cmd/dex/serve.go b/cmd/dex/serve.go
index 6c708c4e..f08e65ee 100644
--- a/cmd/dex/serve.go
+++ b/cmd/dex/serve.go
@@ -136,9 +136,6 @@ func serve(cmd *cobra.Command, args []string) error {
 		if conn.Config == nil {
 			return fmt.Errorf("invalid config: no config field for connector %q", conn.ID)
 		}
-		if conn.Name == "" {
-			return fmt.Errorf("invalid config: no Name field for connector %q", conn.ID)
-		}
 		logger.Infof("config connector: %s", conn.ID)
 
 		connectorLogger := logger.WithField("connector", conn.Name)
diff --git a/server/oauth2.go b/server/oauth2.go
index 18c554ec..15b85ed2 100644
--- a/server/oauth2.go
+++ b/server/oauth2.go
@@ -333,7 +333,10 @@ func (s *Server) newIDToken(clientID string, claims storage.Claims, scopes []str
 
 // parse the initial request from the OAuth2 client.
 func (s *Server) parseAuthorizationRequest(r *http.Request) (req storage.AuthRequest, oauth2Err *authErr) {
-	q := r.URL.Query()
+	if err := r.ParseForm(); err != nil {
+		return req, &authErr{"", "", errInvalidRequest, "Failed to parse request body."}
+	}
+	q := r.Form
 	redirectURI, err := url.QueryUnescape(q.Get("redirect_uri"))
 	if err != nil {
 		return req, &authErr{"", "", errInvalidRequest, "No redirect_uri provided."}


Test patch:
diff --git a/server/oauth2_test.go b/server/oauth2_test.go
index 7f9a449d..83de2256 100644
--- a/server/oauth2_test.go
+++ b/server/oauth2_test.go
@@ -2,8 +2,10 @@ package server
 
 import (
 	"context"
+	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"strings"
 	"testing"
 
 	jose "gopkg.in/square/go-jose.v2"
@@ -17,6 +19,8 @@ func TestParseAuthorizationRequest(t *testing.T) {
 		clients                []storage.Client
 		supportedResponseTypes []string
 
+		usePOST bool
+
 		queryParams map[string]string
 
 		wantErr bool
@@ -37,6 +41,23 @@ func TestParseAuthorizationRequest(t *testing.T) {
 				"scope":         "openid email profile",
 			},
 		},
+		{
+			name: "POST 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",
+			},
+			usePOST: true,
+		},
 		{
 			name: "invalid client id",
 			clients: []storage.Client{
@@ -139,7 +160,14 @@ func TestParseAuthorizationRequest(t *testing.T) {
 				params.Set(k, v)
 			}
 
-			req := httptest.NewRequest("GET", httpServer.URL+"/auth?"+params.Encode(), nil)
+			var req *http.Request
+			if tc.usePOST {
+				body := strings.NewReader(params.Encode())
+				req = httptest.NewRequest("POST", httpServer.URL+"/auth", body)
+				req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+			} else {
+				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)