Prefer HTTP basic authentication in OAuth2 client

spec/std/oauth2/client_spec.cr

@@ -40,79 +40,150 @@ describe OAuth2::Client do
   end
 
   describe "get_access_token_using_*" do
-    it "#get_access_token_using_authorization_code" do
-      server = HTTP::Server.new do |context|
-        body = context.request.body.not_nil!.gets_to_end
-        response = {access_token: "access_token", body: body}
-        context.response.print response.to_json
+    describe "using HTTP Basic authentication to pass credentials" do
+      it "#get_access_token_using_authorization_code" do
+        server = HTTP::Server.new do |context|
+          body = context.request.body.not_nil!.gets_to_end
+          response = {access_token: "access_token", body: body}
+          context.response.print response.to_json
+        end
+
+        address = server.bind_unused_port "::1"
+
+        run_server(server) do
+          client = OAuth2::Client.new "[::1]", "client_id", "client_secret", port: address.port, scheme: "http"
+
+          token = client.get_access_token_using_authorization_code(authorization_code: "SDFhw39fwfg23flSfpawbef")
+          token.extra.not_nil!["body"].should eq %("redirect_uri=&grant_type=authorization_code&code=SDFhw39fwfg23flSfpawbef")
+          token.access_token.should eq "access_token"
+        end
       end
 
-      expected = %("client_id=client_id&client_secret=client_secret&redirect_uri=&grant_type=authorization_code&code=SDFhw39fwfg23flSfpawbef")
-      address = server.bind_unused_port "::1"
+      it "#get_access_token_using_resource_owner_credentials" do
+        server = HTTP::Server.new do |context|
+          body = context.request.body.not_nil!.gets_to_end
+          response = {access_token: "access_token", body: body}
+          context.response.print response.to_json
+        end
 
-      run_server(server) do
-        client = OAuth2::Client.new "[::1]", "client_id", "client_secret", port: address.port, scheme: "http"
+        address = server.bind_unused_port "::1"
 
-        token = client.get_access_token_using_authorization_code(authorization_code: "SDFhw39fwfg23flSfpawbef")
-        token.extra.not_nil!["body"].should eq expected
-        token.access_token.should eq "access_token"
+        run_server(server) do
+          client = OAuth2::Client.new "[::1]", "client_id", "client_secret", port: address.port, scheme: "http"
+
+          token = client.get_access_token_using_resource_owner_credentials(username: "user123", password: "monkey", scope: "read_posts")
+          token.extra.not_nil!["body"].should eq %("grant_type=password&username=user123&password=monkey&scope=read_posts")
+          token.access_token.should eq "access_token"
+        end
       end
-    end
 
-    it "#get_access_token_using_resource_owner_credentials" do
-      server = HTTP::Server.new do |context|
-        body = context.request.body.not_nil!.gets_to_end
-        response = {access_token: "access_token", body: body}
-        context.response.print response.to_json
+      it "#get_access_token_using_client_credentials" do
+        server = HTTP::Server.new do |context|
+          body = context.request.body.not_nil!.gets_to_end
+          response = {access_token: "access_token", body: body}
+          context.response.print response.to_json
+        end
+
+        address = server.bind_unused_port "::1"
+
+        run_server(server) do
+          client = OAuth2::Client.new "[::1]", "client_id", "client_secret", port: address.port, scheme: "http"
+
+          token = client.get_access_token_using_client_credentials(scope: "read_posts")
+          token.extra.not_nil!["body"].should eq %("grant_type=client_credentials&scope=read_posts")
+          token.access_token.should eq "access_token"
+        end
       end
 
-      expected = %("client_id=client_id&client_secret=client_secret&grant_type=password&username=user123&password=monkey&scope=read_posts")
-      address = server.bind_unused_port "::1"
+      it "#get_access_token_using_refresh_token" do
+        server = HTTP::Server.new do |context|
+          body = context.request.body.not_nil!.gets_to_end
+          response = {access_token: "access_token", body: body}
+          context.response.print response.to_json
+        end
+
+        address = server.bind_unused_port "::1"
 
-      run_server(server) do
-        client = OAuth2::Client.new "[::1]", "client_id", "client_secret", port: address.port, scheme: "http"
+        run_server(server) do
+          client = OAuth2::Client.new "[::1]", "client_id", "client_secret", port: address.port, scheme: "http"
 
-        token = client.get_access_token_using_resource_owner_credentials(username: "user123", password: "monkey", scope: "read_posts")
-        token.extra.not_nil!["body"].should eq expected
-        token.access_token.should eq "access_token"
+          token = client.get_access_token_using_refresh_token(scope: "read_posts", refresh_token: "some_refresh_token")
+          token.extra.not_nil!["body"].should eq %("grant_type=refresh_token&refresh_token=some_refresh_token&scope=read_posts")
+          token.access_token.should eq "access_token"
+        end
       end
     end
-
-    it "#get_access_token_using_client_credentials" do
-      server = HTTP::Server.new do |context|
-        body = context.request.body.not_nil!.gets_to_end
-        response = {access_token: "access_token", body: body}
-        context.response.print response.to_json
+    describe "using Request Body to pass credentials" do
+      it "#get_access_token_using_authorization_code" do
+        server = HTTP::Server.new do |context|
+          body = context.request.body.not_nil!.gets_to_end
+          response = {access_token: "access_token", body: body}
+          context.response.print response.to_json
+        end
+
+        address = server.bind_unused_port "::1"
+
+        run_server(server) do
+          client = OAuth2::Client.new "[::1]", "client_id", "client_secret", port: address.port, scheme: "http", auth_scheme: OAuth2::AuthScheme::RequestBody
+
+          token = client.get_access_token_using_authorization_code(authorization_code: "SDFhw39fwfg23flSfpawbef")
+          token.extra.not_nil!["body"].should eq %("client_id=client_id&client_secret=client_secret&redirect_uri=&grant_type=authorization_code&code=SDFhw39fwfg23flSfpawbef")
+          token.access_token.should eq "access_token"
+        end
       end
 
-      expected = %("client_id=client_id&client_secret=client_secret&grant_type=client_credentials&scope=read_posts")
-      address = server.bind_unused_port "::1"
+      it "#get_access_token_using_resource_owner_credentials" do
+        server = HTTP::Server.new do |context|
+          body = context.request.body.not_nil!.gets_to_end
+          response = {access_token: "access_token", body: body}
+          context.response.print response.to_json
+        end
 
-      run_server(server) do
-        client = OAuth2::Client.new "[::1]", "client_id", "client_secret", port: address.port, scheme: "http"
+        address = server.bind_unused_port "::1"
 
-        token = client.get_access_token_using_client_credentials(scope: "read_posts")
-        token.extra.not_nil!["body"].should eq expected
-        token.access_token.should eq "access_token"
+        run_server(server) do
+          client = OAuth2::Client.new "[::1]", "client_id", "client_secret", port: address.port, scheme: "http", auth_scheme: OAuth2::AuthScheme::RequestBody
+
+          token = client.get_access_token_using_resource_owner_credentials(username: "user123", password: "monkey", scope: "read_posts")
+          token.extra.not_nil!["body"].should eq %("client_id=client_id&client_secret=client_secret&grant_type=password&username=user123&password=monkey&scope=read_posts")
+          token.access_token.should eq "access_token"
+        end
       end
-    end
 
-    it "#get_access_token_using_refresh_token" do
-      server = HTTP::Server.new do |context|
-        body = context.request.body.not_nil!.gets_to_end
-        response = {access_token: "access_token", body: body}
-        context.response.print response.to_json
+      it "#get_access_token_using_client_credentials" do
+        server = HTTP::Server.new do |context|
+          body = context.request.body.not_nil!.gets_to_end
+          response = {access_token: "access_token", body: body}
+          context.response.print response.to_json
+        end
+
+        address = server.bind_unused_port "::1"
+
+        run_server(server) do
+          client = OAuth2::Client.new "[::1]", "client_id", "client_secret", port: address.port, scheme: "http", auth_scheme: OAuth2::AuthScheme::RequestBody
+
+          token = client.get_access_token_using_client_credentials(scope: "read_posts")
+          token.extra.not_nil!["body"].should eq %("client_id=client_id&client_secret=client_secret&grant_type=client_credentials&scope=read_posts")
+          token.access_token.should eq "access_token"
+        end
       end
 
-      expected = %("client_id=client_id&client_secret=client_secret&grant_type=refresh_token&refresh_token=some_refresh_token&scope=read_posts")
-      address = server.bind_unused_port "::1"
+      it "#get_access_token_using_refresh_token" do
+        server = HTTP::Server.new do |context|
+          body = context.request.body.not_nil!.gets_to_end
+          response = {access_token: "access_token", body: body}
+          context.response.print response.to_json
+        end
+
+        address = server.bind_unused_port "::1"
 
-      run_server(server) do
-        client = OAuth2::Client.new "[::1]", "client_id", "client_secret", port: address.port, scheme: "http"
+        run_server(server) do
+          client = OAuth2::Client.new "[::1]", "client_id", "client_secret", port: address.port, scheme: "http", auth_scheme: OAuth2::AuthScheme::RequestBody
 
-        token = client.get_access_token_using_refresh_token(scope: "read_posts", refresh_token: "some_refresh_token")
-        token.extra.not_nil!["body"].should eq expected
-        token.access_token.should eq "access_token"
+          token = client.get_access_token_using_refresh_token(scope: "read_posts", refresh_token: "some_refresh_token")
+          token.extra.not_nil!["body"].should eq %("client_id=client_id&client_secret=client_secret&grant_type=refresh_token&refresh_token=some_refresh_token&scope=read_posts")
+          token.access_token.should eq "access_token"
+        end
       end
     end
   end

src/oauth2/auth_scheme.cr

@@ -0,0 +1,15 @@
+# Enum of supported mechanisms used to pass credentials to the server.
+#
+# According to https://tools.ietf.org/html/rfc6749#section-2.3.1:
+#
+# > "Including the client credentials in the request-body using the
+# > two parameters is NOT RECOMMENDED and SHOULD be limited to
+# > clients unable to directly utilize the HTTP Basic authentication
+# > scheme (or other password-based HTTP authentication schemes)."
+#
+# Therefore, HTTP Basic is preferred, and Request Body should only
+# be used if the server does not support HTTP Basic.
+enum OAuth2::AuthScheme
+  HTTPBasic
+  RequestBody
+end

src/oauth2/client.cr

@@ -54,18 +54,27 @@
 # You can also use an `OAuth2::Session` to automatically refresh expired
 # tokens before each request.
 class OAuth2::Client
+  private getter host, client_id, client_secret, port, scheme, authorize_uri,
+    redirect_uri, auth_scheme
+
   # Creates an OAuth client.
   #
   # Any or all of the customizable URIs *authorize_uri* and
   # *token_uri* can be relative or absolute.
   # If they are relative, the given *host*, *port* and *scheme* will be used.
   # If they are absolute, the absolute URL will be used.
+  #
+  # As per https://tools.ietf.org/html/rfc6749#section-2.3.1,
+  # `AuthScheme::HTTPBasic` is the default *auth_scheme* (the mechanism used to
+  # transmit the client credentials to the server). `AuthScheme::RequestBody` should
+  # only be used if the server does not support HTTP Basic.
   def initialize(@host : String, @client_id : String, @client_secret : String,
                  @port : Int32? = nil,
                  @scheme = "https",
                  @authorize_uri = "/oauth2/authorize",
                  @token_uri = "/oauth2/token",
-                 @redirect_uri : String? = nil)
+                 @redirect_uri : String? = nil,
+                 @auth_scheme : AuthScheme = :http_basic)
   end
 
   # Builds an authorize URI, as specified by
@@ -80,16 +89,16 @@ class OAuth2::Client
   # Yields an `HTTP::Params::Builder` to add extra parameters other than those
   # defined by the standard.
   def get_authorize_uri(scope = nil, state = nil, &block : HTTP::Params::Builder ->) : String
-    uri = URI.parse(@authorize_uri)
+    uri = URI.parse(authorize_uri)
 
     # Use the default URI if it's not an absolute one
     unless uri.host
-      uri = URI.new(@scheme, @host, @port, @authorize_uri)
+      uri = URI.new(scheme, host, port, authorize_uri)
     end
 
     uri.query = HTTP::Params.build do |form|
-      form.add "client_id", @client_id
-      form.add "redirect_uri", @redirect_uri
+      form.add "client_id", client_id
+      form.add "redirect_uri", redirect_uri
       form.add "response_type", "code"
       form.add "scope", scope unless scope.nil?
       form.add "state", state unless state.nil?
@@ -108,7 +117,7 @@ class OAuth2::Client
   # [RFC 6749, Section 4.1.3](https://tools.ietf.org/html/rfc6749#section-4.1.3).
   def get_access_token_using_authorization_code(authorization_code : String) : AccessToken
     get_access_token do |form|
-      form.add("redirect_uri", @redirect_uri)
+      form.add("redirect_uri", redirect_uri)
       form.add("grant_type", "authorization_code")
       form.add("code", authorization_code)
     end
@@ -145,18 +154,26 @@ class OAuth2::Client
   end
 
   private def get_access_token : AccessToken
-    body = HTTP::Params.build do |form|
-      form.add("client_id", @client_id)
-      form.add("client_secret", @client_secret)
-      yield form
-    end
-
     headers = HTTP::Headers{
       "Accept"       => "application/json",
       "Content-Type" => "application/x-www-form-urlencoded",
     }
 
-    response = HTTP::Client.post(token_uri, form: body, headers: headers)
+    body = HTTP::Params.build do |form|
+      case auth_scheme
+      when .request_body?
+        form.add("client_id", client_id)
+        form.add("client_secret", client_secret)
+      when .http_basic?
+        headers.add(
+          "Authorization",
+          "Basic #{Base64.strict_encode("#{client_id}:#{client_secret}")}"
+        )
+      end
+      yield form
+    end
+
+    response = HTTP::Client.post token_uri, form: body, headers: headers
     case response.status
     when .ok?, .created?
       OAuth2::AccessToken.from_json(response.body)
@@ -165,15 +182,12 @@ class OAuth2::Client
     end
   end
 
-  private def token_uri
+  private def token_uri : URI
     uri = URI.parse(@token_uri)
-
     if uri.host
-      # If it's an absolute URI, use that one
-      @token_uri
+      uri
     else
-      # Otherwise use the default one
-      URI.new(@scheme, @host, @port, @token_uri).to_s
+      URI.new(scheme, host, port, @token_uri)
     end
   end
 end