[
https://issues.apache.org/jira/browse/SLING-8869?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16988770#comment-16988770
]
Ashish Chopra commented on SLING-8869:
--------------------------------------
{quote}
bq. I still don't see in which case a {{DistributionTransportSecretProvider}}
can be created/updated without a corresponding creation of new instance of
{{RemoteDistributionPackageImporter}}
Any secret provider that changes secrets without deactivating/activating
itself. A concrete example, used in this ticket, is an access token provider.
{quote}
We are talking about different things - I meant changing of the
{{DistributionTransportSecretProvider}} Java implementation object itself,
whereas you meant _contents_ of the {{Map}} returned by
{{DistributionTransportSecretProvider.getSecret().asCredentialsMap()}} changing
(which is what I called "dynamic" secret provider earlier).
But I get your point - there can indeed be a
{{DistributionTransportSecretProvider}} impl which change the username/password
without changing the Java implementation object (reading them off a
secret-store via any API, perhaps).
{quote}
bq. None of the secret-providers can actually 'refresh' the token on a 401
because no such method exists in the API
The implementation parses the response status code and can evict the Executor
from the cache. There would be no need to extend the API.
{quote}
My intent was not to propose extending [the
API|https://github.com/apache/sling-org-apache-sling-distribution-api/blob/master/src/main/java/org/apache/sling/distribution/transport/DistributionTransportSecretProvider.java#L45]
- I wished to point out that _unless_ the
[API|https://github.com/apache/sling-org-apache-sling-distribution-api/blob/master/src/main/java/org/apache/sling/distribution/transport/DistributionTransportSecretProvider.java#L45]
has a 'refresh-secret' method, [evicting the executor upon a 401 v/s calling
{{DistributionTransportSecretProvider.getSecret().asCredentialsMap()}} before
issuing the transport every time will be
equivalent|https://issues.apache.org/jira/browse/SLING-8869?focusedCommentId=16988653&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16988653]
when it comes to freshness/staleness of the retrieved credentials-{{Map}}.
[^SLING-8869-new.patch] follows the latter and benefits from Executor/HTTP
client reuse (this is [in line with @bdelacretaz's suggestion
above|https://issues.apache.org/jira/browse/SLING-8869?focusedCommentId=16986813&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16986813]).
bq. With the approach taken by SLING-8869-new.patch we could reuse the client
and only fix the secret.
Thanks! the corresponding PR is
https://github.com/apache/sling-org-apache-sling-distribution-core/pull/28.
I'll try to update that with a commit applying the executor-eviction on 401
strategy.
\cc: [~mohiaror]
> SimpleHttpDistributionTransport does not refresh the secret for token based
> implementations.
> --------------------------------------------------------------------------------------------
>
> Key: SLING-8869
> URL: https://issues.apache.org/jira/browse/SLING-8869
> Project: Sling
> Issue Type: Bug
> Components: Content Distribution
> Reporter: Mohit Arora
> Assignee: Timothee Maret
> Priority: Critical
> Fix For: Content Distribution Core 0.4.2
>
> Attachments: SLING-8869-new.patch, SLING-8869.patch
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> While saving the {{contextKeyExecutor}} in {{DistributionTransportContext}}
> map, it is not expected that the secret associated with the executor could be
> expired. This can happen in case of access token based implementations where
> the token is expired after a certain period of time and has to be refreshed.
> The code to refresh the token is written in the secret provider but since the
> executor is [cached in the
> map|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/transport/impl/SimpleHttpDistributionTransport.java#L208]
> the secrets are not refreshed. It works fine for credentials based secret
> provider but not for access token based.
> cc - [~marett]
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
