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

@@ -60,12 +60,18 @@ class OAuth2::Client
   # *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
@@ -145,18 +151,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 +179,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