temporalio
Auth interceptor - add ability to pull auth info from custom headers
Currently auth interceptor requires tls or authorization header present: https://github.com/temporalio/temporal/blob/master/common/authorization/interceptor.go#L91
Users would like to be able to define custom headers where this info is present instead.
This feature would be great because it will allow to use custom security headers defined by a company instead of hardcoded "authorization" and "authorization-extras" that is forced today by the code. I would love to configure my custom security header and trigger authorization based on it with or without "authorization" header.
I can provide more details on this if needed.
On Wed, Sep 27, 2023, 11:49 AM Steve Androulakis @.***> wrote:
Any update on this one? Thank you!
— Reply to this email directly, view it on GitHub https://github.com/temporalio/temporal/issues/3362#issuecomment-1737913956, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACZFG6EIESETAXXR2QOGI7DX4RYNNANCNFSM6AAAAAAQJERWSU . You are receiving this because you commented.Message ID: @.***>
Hi @belokrylov, would it be acceptable to allow overriding those two header names (in static config), or do you want the ability to examine an arbitrary number of headers? The latter would require changing the interface which is a little more involved.
Yes, that would be fine.
On Tue, Oct 3, 2023, 1:36 PM David Reiss @.***> wrote:
Hi @belokrylov https://github.com/belokrylov, would it be acceptable to allow overriding those two header names (in static config), or do you want the ability to examine an arbitrary number of headers? The latter would require changing the interface which is a little more involved.
— Reply to this email directly, view it on GitHub https://github.com/temporalio/temporal/issues/3362#issuecomment-1745686047, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACZFG6HDGKDRFBKHPQPXZZLX5RZMDAVCNFSM6AAAAAAQJERWSWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONBVGY4DMMBUG4 . You are receiving this because you were mentioned.Message ID: @.***>
Please check for second header as well. First or second should be checked.
On Wed, Oct 4, 2023, 8:54 AM Alexandre Belokrylov @.***> wrote:
Yes, that would be fine.
On Tue, Oct 3, 2023, 1:36 PM David Reiss @.***> wrote:
Hi @belokrylov https://github.com/belokrylov, would it be acceptable to allow overriding those two header names (in static config), or do you want the ability to examine an arbitrary number of headers? The latter would require changing the interface which is a little more involved.
— Reply to this email directly, view it on GitHub https://github.com/temporalio/temporal/issues/3362#issuecomment-1745686047, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACZFG6HDGKDRFBKHPQPXZZLX5RZMDAVCNFSM6AAAAAAQJERWSWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONBVGY4DMMBUG4 . You are receiving this because you were mentioned.Message ID: @.***>
Could you check if PR #4935 works for you? This lets you add a authHeaderName and authExtraHeaderName in your static config to change the header names to look up.
My developers did review the change. Would it be possible to add into if statement on line 102 code "|| len(authExtraHeaders) >0"
We use authheaders or authextraheaders or both together. Example use case: One is for jwt. Second for client certificate that is passed as a custom header.
On Wed, Oct 4, 2023, 10:38 AM David Reiss @.***> wrote:
Could you check if PR #4935 https://github.com/temporalio/temporal/pull/4935 works for you?
— Reply to this email directly, view it on GitHub https://github.com/temporalio/temporal/issues/3362#issuecomment-1747356526, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACZFG6DYCV3TYQ7C775BK6DX5WNHXAVCNFSM6AAAAAAQJERWSWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONBXGM2TMNJSGY . You are receiving this because you were mentioned.Message ID: @.***>
Would you be able to use this feature?
https://github.com/temporalio/temporal/blob/4be14dd3308031219db02945a12a2cf7d29614b3/common/authorization/claim_mapper.go#L60-L65
You can implement AuthInfoRequired and return false and the ClaimMapper will be called in all cases.
(I know it's a little awkward, but I'm trying to avoid making drastic changes to the interface or semantics to avoid breaking existing custom ClaimMappers.)
Yes. That will work for us! Thank you for bringing this to our attention. We are good with the PR.
Thank you so much for working on it and checking in with us!
On Thu, Oct 5, 2023, 10:26 AM David Reiss @.***> wrote:
Would you be able to use this feature?
https://github.com/temporalio/temporal/blob/4be14dd3308031219db02945a12a2cf7d29614b3/common/authorization/claim_mapper.go#L60-L65 You can implement AuthInfoRequired and return false and the ClaimMapper will be called in all cases.
(I know it's a little awkward, but I'm trying to avoid making drastic changes to the interface or semantics to avoid breaking existing custom ClaimMappers.)
— Reply to this email directly, view it on GitHub https://github.com/temporalio/temporal/issues/3362#issuecomment-1749351717, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACZFG6AA7KAFHFW5FK7FBXDX53UVDAVCNFSM6AAAAAAQJERWSWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONBZGM2TCNZRG4 . You are receiving this because you were mentioned.Message ID: @.***>
Thank you so much!
On Thu, Oct 5, 2023, 3:33 PM David Reiss @.***> wrote:
Closed #3362 https://github.com/temporalio/temporal/issues/3362 as completed via #4935 https://github.com/temporalio/temporal/pull/4935.
— Reply to this email directly, view it on GitHub https://github.com/temporalio/temporal/issues/3362#event-10569164575, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACZFG6C74MH7F52KYXTQUNDX54YRXAVCNFSM6AAAAAAQJERWSWVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJQGU3DSMJWGQ2TONI . You are receiving this because you were mentioned.Message ID: @.***>