Add client registration

Client must now be registered prior to imitating a
call to the token endpoint with the password flow.

Squashed commits:

[fdcef62] Rename authorization endpoint to protected resource endpoint

app/controllers/api/v0/base_controller.rb

@@ -1,5 +1,5 @@
 class Api::V0::BaseController < ApplicationController
-  include OpenidConnect::Authentication
+  include OpenidConnect::ProtectedResourceEndpoint
 
   before_filter :require_access_token
 end

app/controllers/api/v0/users_controller.rb

@@ -6,6 +6,6 @@ class Api::V0::UsersController < Api::V0::BaseController
 
 private
   def user
-    current_token.o_auth_application.user
+    current_token.user
   end
 end

app/controllers/openid_connect/authorizations_controller.rb

@@ -18,7 +18,7 @@ class AuthorizationsController < ApplicationController
   private
 
   def call_authorization_endpoint(is_create = false, approved = false)
-    endpoint = AuthorizationEndpoint.new current_user, is_create, approved
+    endpoint = AuthorizationEndpoint.new is_create, approved
     rack_response = *endpoint.call(request.env)
     @client, @response_type, @redirect_uri, @scopes, @_request_, @request_uri, @request_object = *[
         endpoint.client, endpoint.response_type, endpoint.redirect_uri, endpoint.scopes, endpoint._request_, endpoint.request_uri, endpoint.request_object

app/controllers/openid_connect/clients_controller.rb

@@ -0,0 +1,30 @@
+class OpenidConnect::ClientsController < ApplicationController
+
+  rescue_from OpenIDConnect::HttpError do |e|
+    rewriteHTTPErrorPageAsJSON(e)
+  end
+  rescue_from OpenIDConnect::ValidationFailed do |e|
+    rewriteValidationFailErrorPageAsJSON(e)
+  end
+
+  def create
+    registrar = OpenIDConnect::Client::Registrar.new(request.url, params)
+    client = OAuthApplication.register! registrar
+    render json: client
+  end
+
+private
+
+  def rewriteHTTPErrorPageAsJSON(e)
+    render json: {
+             error: :invalid_request,
+             error_description: e.message
+           }, status: 400
+  end
+  def rewriteValidationFailErrorPageAsJSON(e)
+    render json: {
+             error: :invalid_client_metadata,
+             error_description: e.message
+           }, status: 400
+  end
+end

app/models/o_auth_application.rb

@@ -4,4 +4,32 @@ class OAuthApplication < ActiveRecord::Base
   validates :client_id, presence: true, uniqueness: true
   validates :client_secret, presence: true
 
+  before_validation :setup, on: :create
+  def setup
+    self.client_id = SecureRandom.hex(16)
+    self.client_secret = SecureRandom.hex(32)
+  end
+
+  class << self
+    def register!(registrarHash)
+      registrarHash.validate!
+      buildClientApplication(registrarHash)
+    end
+
+    def buildClientApplication(registrarHash)
+      client = OAuthApplication.create!
+      client.attributes = filterNilValues(registrarHash)
+      client.save!
+      client
+    end
+
+    def filterNilValues(registrarHash)
+      {
+        name: registrarHash.client_name,
+        redirect_uris: registrarHash.redirect_uris
+      }.delete_if do |key, value|
+        value.nil?
+      end
+    end
+  end
 end

features/desktop/oauth_password_flow.feature

@@ -1,19 +1,20 @@
-@javascript
 Feature: Access protected resources using password flow
-  # TODO: Add tests for expired access tokens
   Background:
     Given a user with username "kent"
 
-  Scenario: Valid bearer tokens sent via Authorization Request Header Field
+  Scenario: Invalid credentials to token endpoint
+    When I register a new client
+    And I send a post request from that client to the token endpoint using invalid credentials
+    Then I should receive an "invalid_grant" error
 
-  Scenario: Valid bearer tokens sent via Form Encoded Parameter
+  Scenario: Invalid bearer tokens sent
-
+    When I register a new client
-  Scenario: Valid bearer tokens sent via URI query parameter
+    And I send a post request from that client to the token endpoint using "kent"'s credentials
-    When I send a post request to the token endpoint using "kent"'s credentials
+    And I use invalid bearer tokens to access user info
-    And I use received valid bearer tokens to access user info via URI query parameter
-    Then I should receive "kent"'s id, username, and email
-
-  Scenario: Invalid bearer tokens sent via URI query parameter
-    When I send a post request to the token endpoint using "kent"'s credentials
-    And I use invalid bearer tokens to access user info via URI query parameter
     Then I should receive an "invalid_token" error
+
+  Scenario: Valid password flow
+    When I register a new client
+    And I send a post request from that client to the token endpoint using "kent"'s credentials
+    And I use received valid bearer tokens to access user info
+    Then I should receive "kent"'s id, username, and email

features/step_definitions/openid_steps.rb

@@ -1,37 +1,64 @@
-# Password has been hard coded as all test accounts seem to have a password of "password"
+When /^I register a new client$/ do
-Given /^I send a post request to the token endpoint using "([^\"]*)"'s credentials$/ do |username|
+  clientRegistrationURL = "/openid_connect/clients"
-  user = User.find_by(username: username)
+  post clientRegistrationURL,
-  tokenEndpointURL = "/openid/access_tokens"
+       {
-  tokenEndpointURLQuery = "?grant_type=password&username=" +
+         redirect_uris: ["http://localhost:3000"] # Not actually used
-    user.username +
+       }
-    "&password=password&client_id=4&client_secret=azerty"
-  post tokenEndpointURL + tokenEndpointURLQuery
 end
 
-When /^I use received valid bearer tokens to access user info via URI query parameter$/ do
+Given /^I send a post request from that client to the token endpoint using "([^\"]*)"'s credentials$/ do |username|
+  clientJSON = JSON.parse(last_response.body)
+  user = User.find_by(username: username)
+  tokenEndpointURL = "/openid_connect/access_tokens"
+  post tokenEndpointURL,
+    {
+      grant_type: "password",
+      username: user.username,
+      password: "password", # Password has been hard coded as all test accounts seem to have a password of "password"
+      client_id: clientJSON["o_auth_application"]["client_id"],
+      client_secret: clientJSON["o_auth_application"]["client_secret"]
+    }
+end
+
+Given /^I send a post request from that client to the token endpoint using invalid credentials$/ do
+  clientJSON = JSON.parse(last_response.body)
+  tokenEndpointURL = "/openid_connect/access_tokens"
+  post tokenEndpointURL,
+       {
+         grant_type: "password",
+         username: User.find_by(username: "bob"),
+         password: "wrongpassword",
+         client_id: clientJSON["o_auth_application"]["client_id"],
+         client_secret: clientJSON["o_auth_application"]["client_secret"]
+       }
+end
+
+When /^I use received valid bearer tokens to access user info$/ do
   accessTokenJson = JSON.parse(last_response.body)
   userInfoEndPointURL = "/api/v0/user/"
-  userInfoEndPointURLQuery = "?access_token=" + accessTokenJson["access_token"]
+  get userInfoEndPointURL,
-  visit userInfoEndPointURL + userInfoEndPointURLQuery
+    {
+      access_token: accessTokenJson["access_token"]
+    }
 end
 
-When /^I use invalid bearer tokens to access user info via URI query parameter$/ do
+When /^I use invalid bearer tokens to access user info$/ do
   userInfoEndPointURL = "/api/v0/user/"
-  userInfoEndPointURLQuery = "?access_token=" + SecureRandom.hex(32)
+  get userInfoEndPointURL,
-  visit userInfoEndPointURL + userInfoEndPointURLQuery
+      {
+        access_token: SecureRandom.hex(32)
+      }
 end
 
 Then /^I should receive "([^\"]*)"'s id, username, and email$/ do |username|
+  userInfoJson = JSON.parse(last_response.body)
   user = User.find_by_username(username)
-  expect(page).to have_content(user.username)
+  expect(userInfoJson["username"]).to have_content(user.username)
-  expect(page).to have_content(user.language)
+  expect(userInfoJson["language"]).to have_content(user.language)
-  expect(page).to have_content(user.email)
+  expect(userInfoJson["email"]).to have_content(user.email)
 end
 
 Then /^I should receive an "([^\"]*)" error$/ do |error_message|
-  expect(page).to have_content(error_message)
+  userInfoJson = JSON.parse(last_response.body)
-end
+  expect(userInfoJson["error"]).to have_content(error_message)
-
-Then /^I should see "([^\"]*)" in the content$/ do |content|
-  expect(page).to have_content(content)
 end

lib/openid_connect/authorization/endpoint.rb

@@ -0,0 +1,50 @@
+module OpenidConnect
+  module Authorization
+    class Endpoint
+      attr_accessor :app, :user, :client, :redirect_uri, :response_type, :scopes, :_request_, :request_uri, :request_object
+      delegate :call, to: :app
+
+      def initialize(current_user)
+        @user = current_user
+        @app = Rack::OAuth2::Server::Authorize.new do |req, res|
+          buildClient(req)
+          buildRedirectURI(req, res)
+          verifyNonce(req, res)
+          buildScopes(req)
+          buildRequestObject(req)
+          if OAuthApplication.available_response_types.include? Array(req.response_type).collect(&:to_s).join(' ')
+            handleResponseType(req, res)
+          else
+            req.unsupported_response_type!
+          end
+        end
+      end
+      def buildClient(req)
+        @client = OAuthApplication.find_by_client_id(req.client_id) || req.bad_request!
+      end
+      def buildRedirectURI(req, res)
+        res.redirect_uri = @redirect_uri = req.verify_redirect_uri!(@client.redirect_uris)
+      end
+      def verifyNonce(req, res)
+        if res.protocol_params_location == :fragment && req.nonce.blank?
+          req.invalid_request! 'nonce required'
+        end
+      end
+      def buildScopes(req)
+        @scopes = req.scope.inject([]) do |_scopes_, scope|
+          _scopes_ << Scope.find_by_name(scope) or req.invalid_scope! "Unknown scope: #{scope}"
+        end
+      end
+      def buildRequestObject(req)
+        @request_object = if (@_request_ = req.request).present?
+                            OpenIDConnect::RequestObject.decode req.request, nil # @client.secret
+                          elsif (@request_uri = req.request_uri).present?
+                            OpenIDConnect::RequestObject.fetch req.request_uri, nil # @client.secret
+                          end
+      end
+      def handleResponseType(req, res)
+        # Implemented by subclass
+      end
+    end
+  end
+end

lib/openid_connect/authorization_endpoint.rb

@@ -1,12 +0,0 @@
-module OpenidConnect
-  class AuthorizationEndpoint
-    attr_accessor :app, :account, :client, :redirect_uri, :response_type, :scopes, :_request_, :request_uri, :request_object
-    delegate :call, to: :app
-
-    def initialize(allow_approval = false, approved = false)
-      @app = Rack::OAuth2::Server::Authorize.new do |req, res|
-        req.unsupported_response_type! # TODO: not supported yet
-      end
-    end
-  end
-end

lib/openid_connect/protected_resource_endpoint.rb

@@ -1,8 +1,8 @@
 module OpenidConnect
-  module Authentication
+  module ProtectedResourceEndpoint
 
     def self.included(klass)
-      klass.send :include, Authentication::Helper
+      klass.send :include, ProtectedResourceEndpoint::Helper
     end
 
     module Helper

lib/openid_connect/token_endpoint.rb

@@ -5,35 +5,43 @@ module OpenidConnect
 
     def initialize
       @app = Rack::OAuth2::Server::Token.new do |req, res|
+        o_auth_app = retrieveClient(req)
+        if isAppValid(o_auth_app, req)
+          handleFlows(req, res)
+        else
+          req.invalid_client!
+        end
+      end
+    end
+
+    def handleFlows(req, res)
       case req.grant_type
         when :password
+          handlePasswordFlow(req, res)
+        else
+          req.unsupported_grant_type!
+      end
+    end
+
+    def handlePasswordFlow(req, res)
       user = User.find_for_database_authentication(username: req.username)
       if user
-              o_auth_app = retrieveOrCreateNewClientApplication(req, user)
+        if user.valid_password?(req.password)
-              if o_auth_app && user.valid_password?(req.password)
+          res.access_token = user.tokens.create!.bearer_token
-                res.access_token = o_auth_app.tokens.create!.bearer_token
         else
           req.invalid_grant!
         end
       else
-              req.invalid_grant! # TODO: Change to user login
+        req.invalid_grant! # TODO: Change to user login: Perhaps redirect_to login_path?
-            end
-          else
-            res.unsupported_grant_type!
-        end
       end
     end
 
-    def retrieveOrCreateNewClientApplication(req, user)
+    def retrieveClient(req)
-      retrieveClient(req, user) || createClient(req, user)
+      OAuthApplication.find_by_client_id req.client_id
     end
 
-    def retrieveClient(req, user)
+    def isAppValid(o_auth_app, req)
-      user.o_auth_applications.find_by_client_id req.client_id
+      o_auth_app.client_secret == req.client_secret
-    end
-
-    def createClient(req, user)
-      user.o_auth_applications.create!(client_id: req.client_id, client_secret: req.client_secret)
     end
   end
 end

spec/controllers/openid_connect/clients_controller_spec.rb

@@ -0,0 +1,23 @@
+require 'spec_helper'
+
+describe OpenidConnect::ClientsController, type: :controller do
+  describe "#create" do
+    context "when valid parameters are passed" do
+      it "should return a client id" do
+        post :create,
+             {
+               redirect_uris: ["http://localhost"]
+             }
+        clientJSON = JSON.parse(response.body)
+        expect(clientJSON["o_auth_application"]["client_id"].length).to eq(32)
+      end
+    end
+    context "when redirect uri is missing" do
+      it "should return a invalid_client_metadata error" do
+        post :create
+        clientJSON = JSON.parse(response.body)
+        expect(clientJSON["error"]).to have_content("invalid_client_metadata")
+      end
+    end
+  end
+end

spec/lib/openid_connect/protected_resource_endpoint_spec.rb

@@ -1,20 +1,26 @@
 require 'spec_helper'
 
-describe Api::V0::UsersController, type: :request do
+describe OpenidConnect::ProtectedResourceEndpoint, type: :request do
-  describe "#show" do
+  describe "getting the user info" do
-    let!(:application) { bob.o_auth_applications.create!(client_id: 1, client_secret: "secret") }
+    let!(:token) { bob.tokens.create!.bearer_token.to_s }
-    let!(:token) { application.tokens.create!.bearer_token.to_s }
     let(:invalid_token) { SecureRandom.hex(32).to_s }
+    # TODO: Add tests for expired access tokens
 
-    context "when valid" do
+    context "when access token is valid" do
       it "shows the user's username and email" do
-        get "/api/v0/user/?access_token=" + token
+        get "/api/v0/user/",
+            {
+              access_token: token
+            }
         jsonBody = JSON.parse(response.body)
         expect(jsonBody["username"]).to eq(bob.username)
         expect(jsonBody["email"]).to eq(bob.email)
       end
       it "should include private in the cache-control header" do
-        get "/api/v0/user/?access_token=" + token
+        get "/api/v0/user/",
+            {
+              access_token: token
+            }
         expect(response.headers["Cache-Control"]).to include("private")
       end
     end
@@ -32,15 +38,24 @@ describe Api::V0::UsersController, type: :request do
 
     context "when an invalid access token is provided" do
       it "should respond with a 401 Unauthorized response" do
-        get "/api/v0/user/?access_token=" + invalid_token
+        get "/api/v0/user/",
+            {
+              access_token: invalid_token
+            }
         expect(response.status).to be(401)
       end
       it "should have an auth-scheme value of Bearer" do
-        get "/api/v0/user/?access_token=" + invalid_token
+        get "/api/v0/user/",
+            {
+              access_token: invalid_token
+            }
         expect(response.headers["WWW-Authenticate"]).to include("Bearer")
       end
       it "should contain an invalid_token error" do
-        get "/api/v0/user/?access_token=" + invalid_token
+        get "/api/v0/user/",
+            {
+              access_token: invalid_token
+            }
         expect(response.body).to include("invalid_token")
       end
     end

spec/lib/openid_connect/token_endpoint_spec.rb

@@ -1,63 +1,114 @@
 require 'spec_helper'
 
 describe OpenidConnect::TokenEndpoint, type: :request do
-  describe "password grant type" do
+  let!(:client) { OAuthApplication.create!(redirect_uris: ["http://localhost"]) }
+  describe "the password grant type" do
     context "when the username field is missing" do
       it "should return an invalid request error" do
-        post "/openid/access_tokens?grant_type=password\&password=bluepin7\&client_id=4\&client_secret=azerty"
+        post "/openid_connect/access_tokens",
+             {
+               grant_type: "password",
+               password: "bluepin7",
+               client_id: client.client_id,
+               client_secret: client.client_secret
+             }
         expect(response.body).to include("'username' required")
       end
     end
     context "when the password field is missing" do
       it "should return an invalid request error" do
-        post "/openid/access_tokens?grant_type=password\&username=bob\&client_id=4\&client_secret=azerty"
+        post "/openid_connect/access_tokens",
+             {
+               grant_type: "password",
+               username: "bob",
+               client_id: client.client_id,
+               client_secret: client.client_secret
+             }
         expect(response.body).to include("'password' required")
       end
     end
     context "when the username does not match an existing user" do
       it "should return an invalid request error" do
-        post "/openid/access_tokens?grant_type=password\&username=mewasdfrandom\&password=bluepin7\&client_id=4\&client_secret=azerty"
+        post "/openid_connect/access_tokens",
+             {
+               grant_type: "password",
+               username: "randomnoexist",
+               password: "bluepin7",
+               client_id: client.client_id,
+               client_secret: client.client_secret
+             }
         expect(response.body).to include("invalid_grant")
       end
     end
     context "when the password is invalid" do
       it "should return an invalid request error" do
-        post "/openid/access_tokens?grant_type=password\&username=mewasdfrandom\&password=bluepin7\&client_id=4\&client_secret=azerty"
+        post "/openid_connect/access_tokens",
+             {
+               grant_type: "password",
+               username: "bob",
+               password: "wrongpassword",
+               client_id: client.client_id,
+               client_secret: client.client_secret
+             }
         expect(response.body).to include("invalid_grant")
       end
     end
-    context "when there are duplicate fields" do
-      it "should return an invalid request error" do
-        post "/openid/access_tokens?grant_type=password\&username=bob\&password=bluepin6\&username=bob\&password=bluepin7\&client_id=4\&client_secret=azerty"
-        expect(response.body).to include("invalid_grant")
-        # TODO: Apparently Nov's implementation lets this one pass; however, according to the OIDC spec, we are supposed to reject duplicate fields. Is this a security issue?
-      end
-    end
-    context "when the client is unauthorized" do
-      # TODO: If we support password grant, we should prevent access from unauthorized client applications
-      it "should return an error" do
-        fail
-      end
-    end
-    context "when many unauthorized requests are made" do
-      # TODO: If we support password grant, we should support a way to prevent brute force attacks (using rate-limitation or generating alerts) as specified by RFC 6749 4.3.2 Access Token Request
-      it "should generate an alert" do
-        fail
-      end
-    end
     context "when the request is valid" do
       it "should return an access token" do
-        post "/openid/access_tokens?grant_type=password\&username=bob\&password=bluepin7\&client_id=4\&client_secret=azerty"
+        post "/openid_connect/access_tokens",
+             {
+               grant_type: "password",
+               username: "bob",
+               password: "bluepin7",
+               client_id: client.client_id,
+               client_secret: client.client_secret
+             }
         json = JSON.parse(response.body)
         expect(json["access_token"].length).to eq(64)
         expect(json["token_type"]).to eq("bearer")
         expect(json.keys).to include("expires_in")
       end
     end
+    context "when there are duplicate fields" do
+      it "should return an invalid request error" do
+        post "/openid_connect/access_tokens",
+             {
+               grant_type: "password",
+               username: "bob",
+               password: "bluepin7",
+               username: "bob",
+               password: "bluepin6",
+               client_id: client.client_id,
+               client_secret: client.client_secret
+             }
+        expect(response.body).to include("invalid_grant")
       end
-  describe "unsupported grant type" do
+    end
+    context "when the client is unregistered" do
+      it "should return an error" do
+        post "/openid_connect/access_tokens",
+             {
+               grant_type: "password",
+               username: "bob",
+               password: "bluepin7",
+               client_id: SecureRandom.hex(16).to_s,
+               client_secret: client.client_secret
+             }
+        expect(response.body).to include("invalid_client")
+      end
+    end
+    # TODO: Support a way to prevent brute force attacks using rate-limitation? as specified by RFC 6749 4.3.2 Access Token Request
+  end
+  describe "an unsupported grant type" do
     it "should return an unsupported grant type error" do
-      post "/openid/access_tokens?grant_type=me\&username=bob\&password=bluepin7\&client_id=4\&client_secret=azerty"
+      post "/openid_connect/access_tokens",
+           {
+             grant_type: "noexistgrant",
+             username: "bob",
+             password: "bluepin7",
+             client_id: client.client_id,
+             client_secret: client.client_secret
+           }
       expect(response.body).to include "unsupported_grant_type"
     end
   end