Allow POST requests at authentication endpoint

app/controllers/openid_connect/authorizations_controller.rb

@@ -1,8 +1,7 @@
 class OpenidConnect::AuthorizationsController < ApplicationController
   rescue_from Rack::OAuth2::Server::Authorize::BadRequest do |e|
-    @error = e
-    print e.backtrace[0,10].join("\n")
-    render json: {error: :error, status: e.status} #error_message: e.message
+    logger.info e.backtrace[0,10].join("\n")
+    render json: {error: e.message || :error, status: e.status}
   end
 
   before_action :authenticate_user!
@@ -19,21 +18,34 @@ private
 
   def request_authorization_consent_form
     endpoint = OpenidConnect::Authorization::EndpointStartPoint.new(current_user)
-    endpoint.call(request.env)
-    @client, @response_type, @redirect_uri, @scopes, @request_object = *[
-      endpoint.client, endpoint.response_type, endpoint.redirect_uri, endpoint.scopes, endpoint.request_object
-    ]
-    saveRequestParameters
-    render :new
+    handleStartPointResponse(endpoint)
+  end
+
+  def handleStartPointResponse(endpoint)
+    status, header, response = *endpoint.call(request.env)
+    if response.redirect?
+      redirect_to header['Location']
+    else
+      @client, @response_type, @redirect_uri, @scopes, @request_object = *[
+        endpoint.client, endpoint.response_type, endpoint.redirect_uri, endpoint.scopes, endpoint.request_object
+      ]
+      saveRequestParameters
+      render :new
+    end
   end
 
   def process_authorization_consent(approvedString)
     endpoint = OpenidConnect::Authorization::EndpointConfirmationPoint.new(current_user, to_boolean(approvedString))
     restoreRequestParameters(endpoint)
+    handleConfirmationPointResponse(endpoint)
+  end
+
+  def handleConfirmationPointResponse(endpoint)
     status, header, response = *endpoint.call(request.env)
     redirect_to header['Location']
   end
 
+
   def saveRequestParameters
     session[:client_id], session[:response_type], session[:redirect_uri], session[:scopes], session[:request_object] =
       @client.client_id, @response_type, @redirect_uri, @scopes.collect { |scope| scope.name }, @request_object

config/routes.rb

@@ -236,8 +236,11 @@ Diaspora::Application.routes.draw do
   #OpenID Connect & OAuth
   namespace :openid_connect do
     resources :clients, only: :create
-    resources :authorizations, only: [:new, :create]
     post 'access_tokens', to: proc { |env| OpenidConnect::TokenEndpoint.new.call(env) }
+
+    # Authorization Servers MUST support the use of the HTTP GET and POST methods at the Authorization Endpoint (see http://openid.net/specs/openid-connect-core-1_0.html#AuthResponseValidation).
+    resources :authorizations, only: [:new, :create]
+    post 'authorizations/new', to: 'authorizations#new'
   end
 
   api_version(module: "Api::V0", path: {value: "api/v0"}, default: true) do

lib/openid_connect/authorization/endpoint_start_point.rb

@@ -20,7 +20,7 @@ module OpenidConnect
       end
       def buildScopes(req)
         @scopes = req.scope.inject([]) do |_scopes_, scope|
-          _scopes_ << Scope.find_by_name(scope) or req.invalid_scope! "Unknown scope: #{scope}"
+          _scopes_ << (Scope.find_by_name(scope) or req.invalid_scope! "Unknown scope: #{scope}")
         end
       end
     end

spec/controllers/openid_connect/authorizations_controller_spec.rb

@@ -1,7 +1,8 @@
 require 'spec_helper'
 
 describe OpenidConnect::AuthorizationsController, type: :controller do
-  let!(:client) { OAuthApplication.create!(redirect_uris: ["http://localhost:3000/"]) }
+  let!(:client) { OAuthApplication.create!(name: "Diaspora Test Client", redirect_uris: ["http://localhost:3000/"]) }
+  let!(:client_with_multiple_redirects) { OAuthApplication.create!(name: "Diaspora Test Client", redirect_uris: ["http://localhost:3000/","http://localhost/"]) }
 
   before do
     sign_in :user, alice
@@ -10,23 +11,120 @@ describe OpenidConnect::AuthorizationsController, type: :controller do
   end
 
   describe "#new" do
-    render_views
     context "when valid parameters are passed" do
-      it "should return a form page" do
-        get :new,
+      render_views
+      context "as GET request" do
+        it "should return a form page" do
+          get :new,
+              {
+                client_id: client.client_id,
+                redirect_uri: "http://localhost:3000/",
+                response_type: "id_token",
+                scope: "openid",
+                nonce: SecureRandom.hex(16),
+                state: SecureRandom.hex(16)
+              }
+          expect(response.body).to match("Diaspora Test Client")
+        end
+      end
+      context "as POST request" do
+        it "should return a form page" do
+          post :new,
+              {
+                client_id: client.client_id,
+                redirect_uri: "http://localhost:3000/",
+                response_type: "id_token",
+                scope: "openid",
+                nonce: SecureRandom.hex(16),
+                state: SecureRandom.hex(16)
+              }
+          expect(response.body).to match("Diaspora Test Client")
+        end
+      end
+    end
+    context "when client id is missing" do
+      it "should return an bad request error" do
+        post :new,
              {
-               client_id: client.client_id,
                redirect_uri: "http://localhost:3000/",
                response_type: "id_token",
                scope: "openid",
                nonce: SecureRandom.hex(16),
                state: SecureRandom.hex(16)
              }
-        expect(response.body).to match("Approve")
-        expect(response.body).to match("Deny")
+        expect(response.body).to match("bad_request")
+      end
+    end
+    context "when redirect uri is missing" do
+      context "when only one redirect URL is pre-registered" do
+        it "should return a form pager" do
+          # Note this intentionally behavior diverts from OIDC spec http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
+          # See https://github.com/nov/rack-oauth2/blob/master/lib/rack/oauth2/server/authorize.rb#L63
+          post :new,
+               {
+                 client_id: client.client_id,
+                 response_type: "id_token",
+                 scope: "openid",
+                 nonce: SecureRandom.hex(16),
+                 state: SecureRandom.hex(16)
+               }
+          expect(response.body).to match("Diaspora Test Client")
+        end
+      end
+    end
+    context "when multiple redirect URLs are pre-registered" do
+      it "should return an invalid request error" do
+        post :new,
+             {
+               client_id: client_with_multiple_redirects.client_id,
+               response_type: "id_token",
+               scope: "openid",
+               nonce: SecureRandom.hex(16),
+               state: SecureRandom.hex(16)
+             }
+        expect(response.body).to match("bad_request")
+      end
+    end
+    context "when redirect URI does not match pre-registered URIs" do
+      it "should return an invalid request error" do
+        post :new,
+             {
+               client_id: client.client_id,
+               redirect_uri: "http://localhost:2000/",
+               response_type: "id_token",
+               scope: "openid",
+               nonce: SecureRandom.hex(16)
+             }
+        expect(response.body).to match("bad_request")
+      end
+    end
+    context "when an unsupported scope is passed in" do
+      it "should return an invalid scope error" do
+        post :new,
+             {
+               client_id: client.client_id,
+               redirect_uri: "http://localhost:3000/",
+               response_type: "id_token",
+               scope: "random",
+               nonce: SecureRandom.hex(16),
+               state: SecureRandom.hex(16)
+             }
+        expect(response.body).to match("error=invalid_scope")
+      end
+    end
+    context "when nonce is missing" do
+      it "should return an invalid request error" do
+        post :new,
+             {
+               client_id: client.client_id,
+               redirect_uri: "http://localhost:3000/",
+               response_type: "id_token",
+               scope: "openid",
+               state: SecureRandom.hex(16)
+             }
+        expect(response.location).to match("error=invalid_request")
       end
     end
-    # TODO: Implement tests for missing/invalid parameters
   end
 
   describe "#create" do
@@ -38,16 +136,21 @@ describe OpenidConnect::AuthorizationsController, type: :controller do
             response_type: "id_token",
             scope: "openid",
             nonce: SecureRandom.hex(16),
-            state: SecureRandom.hex(16)
+            state: 4180930983
           }
     end
     context "when authorization is approved" do
-      it "should return the id token in a fragment" do
+      before do
         post :create,
              {
                approve: "true"
              }
-        expect(response.location).to have_content("#id_token=")
+      end
+      it "should return the id token in a fragment" do
+        expect(response.location).to have_content("id_token=")
+      end
+      it "should return the passed in state" do
+        expect(response.location).to have_content("state=4180930983")
       end
     end
     context "when authorization is denied" do
@@ -58,10 +161,10 @@ describe OpenidConnect::AuthorizationsController, type: :controller do
              }
       end
       it "should return an error in the fragment" do
-        expect(response.location).to have_content("#error=")
+        expect(response.location).to have_content("error=")
       end
       it "should NOT contain a id token in the fragment" do
-        expect(response.location).to_not have_content("#id_token=")
+        expect(response.location).to_not have_content("id_token=")
       end
     end
   end