From f4d7f2bfb25aac70715cb8ba7bc024898e856bae Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 25 Jun 2024 15:56:38 +0530 Subject: [PATCH 1/6] [ECO-4845] refactored rest/realtime auth 1. Renamed rest auth extra_auth_headers to client_id_header with both rest/realtime specific impl 2. Added method client_id_header_sync to realtime auth 3. Annotated implementation with right spec --- lib/ably/auth.rb | 31 ++++++++++++++++++------------- lib/ably/realtime/auth.rb | 4 ++++ lib/ably/realtime/connection.rb | 2 +- lib/ably/rest/client.rb | 4 ++-- 4 files changed, 25 insertions(+), 16 deletions(-) diff --git a/lib/ably/auth.rb b/lib/ably/auth.rb index 1e6e03fa6..071d71781 100644 --- a/lib/ably/auth.rb +++ b/lib/ably/auth.rb @@ -411,11 +411,15 @@ def auth_header end # Extra headers that may be used during authentication - # + # spec - RSA7e # @return [Hash] headers - def extra_auth_headers - if client_id && using_basic_auth? - { 'X-Ably-ClientId' => Base64.urlsafe_encode64(client_id) } + def client_id_header(realtime=false) + if options[:client_id] && using_basic_auth? + if realtime + { 'clientId' => options[:client_id] } + else + { 'X-Ably-ClientId' => Base64.urlsafe_encode64(options[:client_id]) } + end else {} end @@ -482,15 +486,16 @@ def can_assume_client_id?(assumed_client_id) # # @api private def configure_client_id(new_client_id) - # If new client ID from Ably is a wildcard, but preconfigured clientId is set, then keep the existing clientId - if has_client_id? && new_client_id == '*' - @client_id_validated = true - return - end - - # If client_id is defined and not a wildcard, prevent it changing, this is not supported - if client_id && client_id != '*' && new_client_id != client_id - raise Ably::Exceptions::IncompatibleClientId.new("Client ID is immutable once configured for a client. Client ID cannot be changed to '#{new_client_id}'") + if has_client_id? + # If new client ID from Ably is a wildcard, but preconfigured clientId is set, then keep the existing clientId + if new_client_id == "*" + @client_id_validated = true + return + end + # If client_id is defined and not a wildcard, prevent it changing, this is not supported + if new_client_id != client_id + raise Ably::Exceptions::IncompatibleClientId.new("Client ID is immutable once configured for a client. Client ID cannot be changed to '#{new_client_id}'") + end end @client_id_validated = true @client_id = new_client_id diff --git a/lib/ably/realtime/auth.rb b/lib/ably/realtime/auth.rb index 4d12f87a1..c73b2635f 100644 --- a/lib/ably/realtime/auth.rb +++ b/lib/ably/realtime/auth.rb @@ -226,6 +226,10 @@ def auth_header_sync auth_sync.auth_header end + def client_id_header_sync + auth_sync.client_id_header(true) + end + # Auth params used in URI endpoint for Realtime connections # Will reauthorize implicitly if required and capable # diff --git a/lib/ably/realtime/connection.rb b/lib/ably/realtime/connection.rb index 008a21164..185d60657 100644 --- a/lib/ably/realtime/connection.rb +++ b/lib/ably/realtime/connection.rb @@ -470,7 +470,7 @@ def create_websocket_transport 'false' end - url_params['clientId'] = client.auth.client_id if client.auth.has_client_id? + url_params.merge!(client.auth.client_id_header_sync) # RSA7e1 url_params.merge!(client.transport_params) if !key.nil_or_empty? and connection_state_available? diff --git a/lib/ably/rest/client.rb b/lib/ably/rest/client.rb index 7aa34e3cb..14fe66e76 100644 --- a/lib/ably/rest/client.rb +++ b/lib/ably/rest/client.rb @@ -597,8 +597,8 @@ def send_request(method, path, params, options) end unless options[:send_auth_header] == false request.headers[:authorization] = auth.auth_header - - options[:headers].to_h.merge(auth.extra_auth_headers).map do |key, val| + # RSA7e2 + options[:headers].to_h.merge(auth.client_id_header).map do |key, val| request.headers[key] = val end end From bb4b71f0abe7e17d58d1d10562fb7fd70a66f351 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 25 Jun 2024 16:13:01 +0530 Subject: [PATCH 2/6] Fixed naming convention for client_id_header as per review comment --- lib/ably/auth.rb | 4 ++-- lib/ably/realtime/auth.rb | 2 +- lib/ably/realtime/connection.rb | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/ably/auth.rb b/lib/ably/auth.rb index 071d71781..1fc3f0e3f 100644 --- a/lib/ably/auth.rb +++ b/lib/ably/auth.rb @@ -413,9 +413,9 @@ def auth_header # Extra headers that may be used during authentication # spec - RSA7e # @return [Hash] headers - def client_id_header(realtime=false) + def client_id_header(realtime_params=false) if options[:client_id] && using_basic_auth? - if realtime + if realtime_params { 'clientId' => options[:client_id] } else { 'X-Ably-ClientId' => Base64.urlsafe_encode64(options[:client_id]) } diff --git a/lib/ably/realtime/auth.rb b/lib/ably/realtime/auth.rb index c73b2635f..b36ab9c22 100644 --- a/lib/ably/realtime/auth.rb +++ b/lib/ably/realtime/auth.rb @@ -226,7 +226,7 @@ def auth_header_sync auth_sync.auth_header end - def client_id_header_sync + def client_id_params_sync auth_sync.client_id_header(true) end diff --git a/lib/ably/realtime/connection.rb b/lib/ably/realtime/connection.rb index 185d60657..3d5fc31bc 100644 --- a/lib/ably/realtime/connection.rb +++ b/lib/ably/realtime/connection.rb @@ -470,7 +470,7 @@ def create_websocket_transport 'false' end - url_params.merge!(client.auth.client_id_header_sync) # RSA7e1 + url_params.merge!(client.auth.client_id_params_sync) # RSA7e1 url_params.merge!(client.transport_params) if !key.nil_or_empty? and connection_state_available? From c10e485344ac4f87e7d83445a6ab3b9b287f1129 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 25 Jun 2024 16:23:22 +0530 Subject: [PATCH 3/6] Removed unncessary use of auth clientId while entering presence local members --- lib/ably/realtime/presence/members_map.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/ably/realtime/presence/members_map.rb b/lib/ably/realtime/presence/members_map.rb index 49b006f6f..de3bcebcb 100644 --- a/lib/ably/realtime/presence/members_map.rb +++ b/lib/ably/realtime/presence/members_map.rb @@ -228,13 +228,11 @@ def setup_event_handlers def enter_local_members local_members.values.each do |member| - local_client_id = member.client_id || client.auth.client_id - logger.debug { "#{self.class.name}: Manually re-entering local presence member, client ID: #{local_client_id} with data: #{member.data}" } - presence.enter_client_with_id(member.id, local_client_id, member.data).tap do |deferrable| + logger.debug { "#{self.class.name}: Manually re-entering local presence member, client ID: #{member.client_id} with data: #{member.data}" } + presence.enter_client_with_id(member.id, member.client_id, member.data).tap do |deferrable| deferrable.errback do |error| - presence_message_client_id = member.client_id || client.auth.client_id re_enter_error = Ably::Models::ErrorInfo.new( - message: "unable to automatically re-enter presence channel for client_id '#{presence_message_client_id}'. Source error code #{error.code} and message '#{error.message}'", + message: "unable to automatically re-enter presence channel for client_id '#{member.client_id}'. Source error code #{error.code} and message '#{error.message}'", code: Ably::Exceptions::Codes::UNABLE_TO_AUTOMATICALLY_REENTER_PRESENCE_CHANNEL ) channel.emit :update, Ably::Models::ChannelStateChange.new( From 0f49edecd66af3e27b7fc5efb75c871e0dccf647 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 25 Jun 2024 16:29:04 +0530 Subject: [PATCH 4/6] renamed auth client_id_header to external_client_id --- lib/ably/auth.rb | 4 ++-- lib/ably/realtime/auth.rb | 4 ++-- lib/ably/realtime/connection.rb | 2 +- lib/ably/rest/client.rb | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/ably/auth.rb b/lib/ably/auth.rb index 1fc3f0e3f..b654ea79a 100644 --- a/lib/ably/auth.rb +++ b/lib/ably/auth.rb @@ -413,9 +413,9 @@ def auth_header # Extra headers that may be used during authentication # spec - RSA7e # @return [Hash] headers - def client_id_header(realtime_params=false) + def external_client_id(realtime=false) if options[:client_id] && using_basic_auth? - if realtime_params + if realtime { 'clientId' => options[:client_id] } else { 'X-Ably-ClientId' => Base64.urlsafe_encode64(options[:client_id]) } diff --git a/lib/ably/realtime/auth.rb b/lib/ably/realtime/auth.rb index b36ab9c22..55b1168f9 100644 --- a/lib/ably/realtime/auth.rb +++ b/lib/ably/realtime/auth.rb @@ -226,8 +226,8 @@ def auth_header_sync auth_sync.auth_header end - def client_id_params_sync - auth_sync.client_id_header(true) + def external_client_id_sync + auth_sync.external_client_id(true) end # Auth params used in URI endpoint for Realtime connections diff --git a/lib/ably/realtime/connection.rb b/lib/ably/realtime/connection.rb index 3d5fc31bc..12669e8ec 100644 --- a/lib/ably/realtime/connection.rb +++ b/lib/ably/realtime/connection.rb @@ -470,7 +470,7 @@ def create_websocket_transport 'false' end - url_params.merge!(client.auth.client_id_params_sync) # RSA7e1 + url_params.merge!(client.auth.external_client_id_sync) # RSA7e1 url_params.merge!(client.transport_params) if !key.nil_or_empty? and connection_state_available? diff --git a/lib/ably/rest/client.rb b/lib/ably/rest/client.rb index 14fe66e76..e1e18aefd 100644 --- a/lib/ably/rest/client.rb +++ b/lib/ably/rest/client.rb @@ -598,7 +598,7 @@ def send_request(method, path, params, options) unless options[:send_auth_header] == false request.headers[:authorization] = auth.auth_header # RSA7e2 - options[:headers].to_h.merge(auth.client_id_header).map do |key, val| + options[:headers].to_h.merge(auth.external_client_id).map do |key, val| request.headers[key] = val end end From 1ff7d6b7877362714542a6fecd70654fa6136941 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Wed, 3 Jul 2024 17:35:50 +0530 Subject: [PATCH 5/6] Added separate method client_id_for_request that returns client_id --- lib/ably/auth.rb | 19 +++++++++++-------- lib/ably/realtime/auth.rb | 4 ++-- lib/ably/realtime/connection.rb | 4 ++-- lib/ably/rest/client.rb | 2 +- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/lib/ably/auth.rb b/lib/ably/auth.rb index b654ea79a..0befcb27f 100644 --- a/lib/ably/auth.rb +++ b/lib/ably/auth.rb @@ -411,20 +411,23 @@ def auth_header end # Extra headers that may be used during authentication - # spec - RSA7e + # # @return [Hash] headers - def external_client_id(realtime=false) - if options[:client_id] && using_basic_auth? - if realtime - { 'clientId' => options[:client_id] } - else - { 'X-Ably-ClientId' => Base64.urlsafe_encode64(options[:client_id]) } - end + def extra_auth_headers + if client_id_for_request + { 'X-Ably-ClientId' => Base64.urlsafe_encode64(client_id_for_request) } else {} end end + # ClientId that needs to be included with every rest/realtime request + # spec - RSA7e + # @return string + def client_id_for_request + options[:client_id] if options[:client_id] && using_basic_auth? + end + # Auth params used in URI endpoint for Realtime connections # Will reauthorize implicitly if required and capable # diff --git a/lib/ably/realtime/auth.rb b/lib/ably/realtime/auth.rb index 55b1168f9..c1fc59831 100644 --- a/lib/ably/realtime/auth.rb +++ b/lib/ably/realtime/auth.rb @@ -226,8 +226,8 @@ def auth_header_sync auth_sync.auth_header end - def external_client_id_sync - auth_sync.external_client_id(true) + def client_id_for_request_sync + auth_sync.client_id_for_request end # Auth params used in URI endpoint for Realtime connections diff --git a/lib/ably/realtime/connection.rb b/lib/ably/realtime/connection.rb index 12669e8ec..c508ce92c 100644 --- a/lib/ably/realtime/connection.rb +++ b/lib/ably/realtime/connection.rb @@ -469,8 +469,8 @@ def create_websocket_transport else 'false' end - - url_params.merge!(client.auth.external_client_id_sync) # RSA7e1 + # RSA7e1 + url_params['clientId'] = client.auth.client_id_for_request_sync if client.auth.client_id_for_request_sync url_params.merge!(client.transport_params) if !key.nil_or_empty? and connection_state_available? diff --git a/lib/ably/rest/client.rb b/lib/ably/rest/client.rb index e1e18aefd..3dbda2dfb 100644 --- a/lib/ably/rest/client.rb +++ b/lib/ably/rest/client.rb @@ -598,7 +598,7 @@ def send_request(method, path, params, options) unless options[:send_auth_header] == false request.headers[:authorization] = auth.auth_header # RSA7e2 - options[:headers].to_h.merge(auth.external_client_id).map do |key, val| + options[:headers].to_h.merge(auth.extra_auth_headers).map do |key, val| request.headers[key] = val end end From 91e5f29b8324196225b113ce81b4fdd441780b67 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Wed, 3 Jul 2024 17:37:48 +0530 Subject: [PATCH 6/6] Revert "Removed unncessary use of auth clientId while entering presence local members" This reverts commit c10e485344ac4f87e7d83445a6ab3b9b287f1129. --- lib/ably/realtime/presence/members_map.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/ably/realtime/presence/members_map.rb b/lib/ably/realtime/presence/members_map.rb index de3bcebcb..49b006f6f 100644 --- a/lib/ably/realtime/presence/members_map.rb +++ b/lib/ably/realtime/presence/members_map.rb @@ -228,11 +228,13 @@ def setup_event_handlers def enter_local_members local_members.values.each do |member| - logger.debug { "#{self.class.name}: Manually re-entering local presence member, client ID: #{member.client_id} with data: #{member.data}" } - presence.enter_client_with_id(member.id, member.client_id, member.data).tap do |deferrable| + local_client_id = member.client_id || client.auth.client_id + logger.debug { "#{self.class.name}: Manually re-entering local presence member, client ID: #{local_client_id} with data: #{member.data}" } + presence.enter_client_with_id(member.id, local_client_id, member.data).tap do |deferrable| deferrable.errback do |error| + presence_message_client_id = member.client_id || client.auth.client_id re_enter_error = Ably::Models::ErrorInfo.new( - message: "unable to automatically re-enter presence channel for client_id '#{member.client_id}'. Source error code #{error.code} and message '#{error.message}'", + message: "unable to automatically re-enter presence channel for client_id '#{presence_message_client_id}'. Source error code #{error.code} and message '#{error.message}'", code: Ably::Exceptions::Codes::UNABLE_TO_AUTOMATICALLY_REENTER_PRESENCE_CHANNEL ) channel.emit :update, Ably::Models::ChannelStateChange.new(