Skip to content
This repository has been archived by the owner on Jun 2, 2024. It is now read-only.

Commit

Permalink
Make nonce stick to the session
Browse files Browse the repository at this point in the history
It should be tied to the session. Will get rid of most nonce validation issues stemming from the users using the back-button or multiple tabs.
Version bump
  • Loading branch information
ondrejpialek committed Apr 6, 2022
1 parent 53aed93 commit c2e579b
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 76 deletions.
3 changes: 3 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ Metrics/BlockLength:
Metrics/MethodLength:
Max: 45

Metrics/ClassLength:
Max: 500

AllCops:
Exclude:
- bin/**/*
Expand Down
2 changes: 1 addition & 1 deletion lib/omniauth/openid_connect/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@

module OmniAuth
module OpenIDConnect
VERSION = '0.9.8'
VERSION = '0.9.9'
end
end
132 changes: 57 additions & 75 deletions lib/omniauth/strategies/openid_connect.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
# -*- coding:utf-8 -*-
# frozen_string_literal: true

#require 'addressable/uri'
# require 'addressable/uri'
require 'timeout'
require 'net/http'
require 'omniauth'
require 'openid_connect'
#require 'jwt'
# require 'jwt'
require 'forwardable'

module OmniAuth
Expand Down Expand Up @@ -114,9 +113,9 @@ class OpenIDConnect
option :max_age

# not option, but by request.params
#option :ui_locales
#option :id_token_hint
#option :login_hint
# option :ui_locales
# option :id_token_hint
# option :login_hint

# Authentication Request: [OPTIONAL]
option :acr_values
Expand All @@ -127,7 +126,7 @@ class OpenIDConnect
option :send_nonce, true

# Must verify the id_token. So remove this option.
#option :verify_id_token, nil
# option :verify_id_token, nil

option :ux

Expand Down Expand Up @@ -199,8 +198,8 @@ def uid
image: user_info.picture,
phone: user_info.phone_number,
urls: {
website: user_info.website
}
website: user_info.website,
},
}
end

Expand All @@ -209,7 +208,7 @@ def uid
{}
else
{ raw_info: fix_user_info(user_info).raw_attributes }
end
end
end

credentials do
Expand All @@ -221,7 +220,7 @@ def uid
token: access_token.access_token,
refresh_token: access_token.refresh_token,
expires_in: access_token.expires_in,
scope: access_token.scope
scope: access_token.scope,
}
end
end
Expand All @@ -239,9 +238,7 @@ def initialize(app, *args, &block)
# client_options から OpenIDConnect::Client インスタンスを構築.
# @return [OpenIDConnect::Client] サーバとのconnection
def client
if !client_options.host
raise TypeError, 'internal error: call setup_phase() first'
end
raise TypeError, 'internal error: call setup_phase() first' unless client_options.host
@client ||= ::OpenIDConnect::Client.new(client_options)
end

Expand All @@ -261,14 +258,14 @@ def config
# request_phase() と callback_phase() の開始前に呼び出される.
def setup_phase
super
raise TypeError if !options.issuer.is_a?(String)
raise TypeError unless options.issuer.is_a?(String)
unless (uri = URI.parse(options.issuer)) &&
%w[http https].include?(uri.scheme)
%w[http https].include?(uri.scheme)
raise ArgumentError, 'Invalid issuer URI scheme'
end
@issuer = options.issuer

if !client_options.host
unless client_options.host
client_options.scheme = uri.scheme
client_options.host = uri.host
client_options.port = uri.port
Expand All @@ -282,7 +279,7 @@ def setup_phase
discover! if options.discovery

if configured_response_type != 'code' &&
configured_response_type != 'id_token token'
configured_response_type != 'id_token token'
raise ArgumentError, 'Invalid response_type'
end
if configured_response_type == 'id_token token'
Expand Down Expand Up @@ -313,16 +310,16 @@ def callback_phase
error_description =
params['error_description'] || params['error_reason']
raise CallbackError.new(
params['error'],
error_description, # optional
params['error_uri']
) # optional
params['error'],
error_description, # optional
params['error_uri']
) # optional
end
if session['omniauth.state'] &&
(
params['state'].to_s.empty? ||
params['state'] != session.delete('omniauth.state')
)
(
params['state'].to_s.empty? ||
params['state'] != session.delete('omniauth.state')
)
# RFC 6749 4.1.2: クライアントからの認可リクエストに stateパラメータ
# が含まれていた場合は, そのまま返ってくる. [REQUIRED]
raise CallbackError.new(:csrf_detected, "Invalid 'state' parameter")
Expand Down Expand Up @@ -366,9 +363,9 @@ def other_phase
call_app!
end

#def authorization_code ... params.delete() しないといかん. Remove.
# def authorization_code ... params.delete() しないといかん. Remove.
# params['code']
#end
# end

def end_session_uri
return unless end_session_endpoint_is_valid?
Expand All @@ -391,19 +388,12 @@ def authorize_params
# Others
state: new_state,
response_mode: options.response_mode,
nonce:
(
if options.send_nonce ||
configured_response_type == 'id_token token'
new_nonce
end
),
hd: options.hd
nonce: nonce(options.send_nonce ||
configured_response_type == 'id_token token'),
hd: options.hd,
}

unless options.extra_authorize_params.empty?
opts.merge!(options.extra_authorize_params)
end
opts.merge!(options.extra_authorize_params) unless options.extra_authorize_params.empty?

# Optional params.
%i[display max_age acr_values ux].each do |key|
Expand All @@ -420,10 +410,10 @@ def authorize_params
'email',
'realm',
'cid',
'chem'
'chem',
].each { |key| opts[key.to_sym] = params[key] if params[key] }

return opts.reject { |_k, v| v.nil? }
opts.reject { |_k, v| v.nil? }
end

# @return [JSON::JWK::Set or JSON::JWK] IdP's RSA public keys. NOT client's.
Expand Down Expand Up @@ -460,7 +450,7 @@ def public_key(kid = nil)
attr_reader :issuer

def discover!
raise 'internal bug' if !options.discovery
raise 'internal bug' unless options.discovery

# config() 内で, issuer を引数にして, 実際に discover! している.
client_options.authorization_endpoint = config.authorization_endpoint
Expand All @@ -469,22 +459,20 @@ def discover!

# OpenIDConnect::Discovery::Provider::Config::Response に expires_in は
# ない.
#client_options.expires_in = config().expires_in
# client_options.expires_in = config().expires_in

# client_options に jwks_uri, end_session_endpoint はない.
options.jwks_uri = config.jwks_uri
if config.respond_to?(:end_session_endpoint)
options.end_session_endpoint = config.end_session_endpoint
end
options.end_session_endpoint = config.end_session_endpoint if config.respond_to?(:end_session_endpoint)

if config.respond_to?(:token_endpoint_auth_methods_supported)
if config.token_endpoint_auth_methods_supported.include?(
'client_secret_basic'
)
'client_secret_basic'
)
options.client_auth_method = :basic
elsif config.token_endpoint_auth_methods_supported.include?(
'client_secret_post'
)
'client_secret_post'
)
options.client_auth_method = :secret_in_body
end
end
Expand All @@ -506,25 +494,23 @@ def user_info
return @user_info if @user_info

# openidconnect package: openid_connect/access_token.rb
return {} if !access_token
if !access_token.is_a?(::OpenIDConnect::AccessToken)
raise TypeError, 'internal error'
end
return {} unless access_token
raise TypeError, 'internal error' unless access_token.is_a?(::OpenIDConnect::AccessToken)
@user_info = access_token.userinfo!
return @user_info
@user_info
end

# Google sends the string "true" as the value for the field
# 'email_verified' while a boolean is expected.
def fix_user_info(user_info)
raise TypeError if !user_info
raise TypeError unless user_info

if user_info.email_verified.is_a? String
user_info.email_verified =
(user_info.email_verified.casecmp('true') == 0)
end

#user_info.gender = nil # in case someone picks something else than male or female, we don't need it anyway
# user_info.gender = nil # in case someone picks something else than male or female, we don't need it anyway
user_info
end

Expand All @@ -548,12 +534,10 @@ def authorization_code_flow_callback_phase
# 仕様では grant_type, code, redirect_uri パラメータ
opts = {
scope: (options.scope if options.send_scope_to_token_endpoint),
client_auth_method: options.client_auth_method
client_auth_method: options.client_auth_method,
}
@access_token = client.access_token! opts
if !@access_token.is_a?(Rack::OAuth2::AccessToken)
raise TypeError, 'internal error'
end
raise TypeError, 'internal error' unless @access_token.is_a?(Rack::OAuth2::AccessToken)

# 鍵を選ぶ。"{ヘッダ部}.{ペイロード部}.{シグネチャ部}" と、ピリオドで
# 区切られている。ヘッダ部にアルゴリズムが書かれている.
Expand All @@ -563,7 +547,7 @@ def authorization_code_flow_callback_phase
:skip_verification
).header

#header = ::JWT.decoded_segments(actoken.id_token, false)[0]
# header = ::JWT.decoded_segments(actoken.id_token, false)[0]
key = key_or_secret header

# このなかで署名の検証も行う. => JSON::JWS::VerificationFailed
Expand All @@ -582,7 +566,7 @@ def authorization_code_flow_callback_phase
# [Security issue] On Authorization Code Flow, use key_or_secret, not
# public_key. On the other hand, you have to use public_key on Implicit
# Flow.
#def decode_id_token(id_token)
# def decode_id_token(id_token)

def client_options
options.client_options
Expand All @@ -600,8 +584,8 @@ def new_state
session['omniauth.state'] = state || SecureRandom.hex(16)
end

def new_nonce
session['omniauth.nonce'] = SecureRandom.hex(16)
def nonce(force = false)
session['omniauth.nonce'] ||= force ? SecureRandom.hex(16) : nil
end

# @override
Expand All @@ -616,7 +600,7 @@ def session
# HMAC-SHA256 の場合は, client_secret を共通鍵とする
# RSAの場合は, 認証サーバの公開鍵を使う
def key_or_secret(header)
raise TypeError if !header
raise TypeError unless header

case header['alg'].to_sym
when :HS256, :HS384, :HS512
Expand All @@ -632,15 +616,15 @@ def key_or_secret(header)
end

# [Security issue] Do not use params['redirect_uri']
#def redirect_uri
# def redirect_uri

# @return [String] クエリパラメータ.
def encoded_post_logout_redirect_uri
return nil unless options.post_logout_redirect_uri

# post_logout_redirect_uri を指定する場合は, id_token_hint 必須.
URI.encode_www_form(
# id_token_hint: raise('Not Implemented Error'), # TODO: impl. [REQUIRED] hintなのに必須とはどういうこと?
# id_token_hint: raise('Not Implemented Error'), # TODO: impl. [REQUIRED] hintなのに必須とはどういうこと?
post_logout_redirect_uri: options.post_logout_redirect_uri
)
end
Expand Down Expand Up @@ -690,7 +674,7 @@ def implicit_flow_callback_phase
end

def configured_response_type
if !@configured_response_type
unless @configured_response_type
ary =
case options.response_type
when Array
Expand All @@ -702,17 +686,15 @@ def configured_response_type
end
@configured_response_type = ary.sort.join(' ')
end
return @configured_response_type
@configured_response_type
end

def verify_id_token!(decoded_id_token)
if !decoded_id_token.is_a?(::OpenIDConnect::ResponseObject::IdToken)
raise TypeError
end
raise TypeError unless decoded_id_token.is_a?(::OpenIDConnect::ResponseObject::IdToken)
decoded_id_token.verify!(
issuer: issuer,
client_id: client_options.identifier,
nonce: session.delete('omniauth.nonce')
nonce: nonce
)
end

Expand All @@ -724,7 +706,7 @@ class CallbackError < StandardError
# @param error_reason [OPTIONAL] Human-readable text.
# @param error_uri [OPTIONAL]
def initialize(error, error_reason = nil, error_uri = nil)
raise TypeError if !error
raise TypeError unless error

@error = error
@error_reason = error_reason
Expand Down

0 comments on commit c2e579b

Please sign in to comment.