Add support for Trusted Profile authentication #12
Conversation
Signed-off-by: Suruthi Ganesan Kalavathy <[email protected]>
Signed-off-by: Suruthi Ganesan Kalavathy <[email protected]>
Co-authored-by: shahulsonhal <[email protected]> Signed-off-by: Suruthi Ganesan Kalavathy <[email protected]>
Suruthi-G-K
force-pushed
the
trusted-profile
branch
from
71c9f31 to
8112659
Compare
Signed-off-by: shahulsonhal <[email protected]>
shahulsonhal
force-pushed
the
trusted-profile
branch
from
09353bd to
bb8f768
Compare
- Update retrieve function to fetch token only if it is expired. Replaced `RequestToken` with `GetToken` function. - Add test case for trusted profile retrieve function Co-authored-by: Suruthi-G-K <[email protected]> Signed-off-by: shahulsonhal <[email protected]>
shahulsonhal
force-pushed
the
trusted-profile
branch
from
50f64b8 to
783a3b4
Compare
| // - Expires in (terms of seconds) | ||
| // - Expiration time | ||
| // Error object | ||
| // |
| // Manadatory Refresh Timeout | ||
| // Timer | ||
| // Token Manager Client Do Operation | ||
| // |
There was a problem hiding this comment.
can you avoid also this change?
|
|
||
| file, err := ioutil.TempFile(os.TempDir(), "crtoken") | ||
| require.NoError(t, err) | ||
| fmt.Println(file.Name()) |
There was a problem hiding this comment.
we don't need to print the file name.
There was a problem hiding this comment.
Committed by mistake, removed.
| @@ -281,7 +342,8 @@ func (tmm *tokenManagerMock2) Get() (*token.Token, error) { | |||
|
|
|||
| // Mock Token Manager Two Constructor | |||
| // Returns: | |||
| // Mock Token Manager with IBM IAM Authentication Server Endpoint | |||
| // | |||
| // Mock Token Manager with IBM IAM Authentication Server Endpoint | |||
There was a problem hiding this comment.
could you also avoid this change?
| // Trusted Profile Name | ||
| // Compute Resource Token File Path | ||
| // IBM IAM Authentication Server Endpoint | ||
| // Service Instance ID |
There was a problem hiding this comment.
wondering why we need service instance id here?
| return provider | ||
| } | ||
|
|
||
| if trustedProfileName == "" && trustedProfileID == "" { |
There was a problem hiding this comment.
we can use the TP name, ID. Something we need to be clear of in the docs/comments.
There was a problem hiding this comment.
True. As far as I understood, the go-sdk-core does not have an implementation to support the profile_crn.
| // | ||
| // TrustedProfileProvider expired or not - boolean | ||
| func (p *TrustedProfileProvider) IsExpired() bool { | ||
| return true |
There was a problem hiding this comment.
why it is always true but not false?
There was a problem hiding this comment.
The GetToken function is actually checking the expiration before it's trying to get the token from server. Also the token in authenticator is not accessible in IsExpired function since it is not an exported variable and we won't be able to check the expiration from our side. I think it should be okay to return true since that logic is handled inside the GetToken function. Also updated the doc comment.
shahulsonhal
force-pushed
the
trusted-profile
branch
2 times, most recently
from
bbba477 to
1adc469
Compare
| // Provider Name | ||
| // AWS Config | ||
| // Trusted Profile Name | ||
| // Service Profile ID |
There was a problem hiding this comment.
I think you mean Trusted Profile ID.
Signed-off-by: shahulsonhal <[email protected]>
shahulsonhal
force-pushed
the
trusted-profile
branch
from
1adc469 to
7b793c2
Compare
|
|
||
| authenticator, err := core.NewContainerAuthenticatorBuilder(). | ||
| SetIAMProfileName(trustedProfileName). | ||
| SetIAMProfileID(trustedProfileID). |
There was a problem hiding this comment.
What happens if you only set the name, but not the id, as far as I know, it is sufficient to either use the name or ID.
There was a problem hiding this comment.
Yes it is sufficient to set either ID/TP name. We tested this logic using Unit test case and we were able to authenticate using either one of them
There was a problem hiding this comment.
You are correct. We added profile id to just give user a choice over name or id. It will work:
- if we set the profile name only
- if we set the profile id only
- if we set both it should be of the same trusted profile, otherwise authentication will fail.
What this PR does / why we need it:
Added logic to support authentication using Trusted Profile. We are integrating COS as one of a storage provider in Loki. As part of this we are using
ibm-cos-sdk-gopackage to communicate to COS. We want to authenticate to COS using Trusted Profile instead of Apikey so that we don't have to manage/rotate the Apikey.Notes:
We created a new provider for trusted profile that is similar to the existing static provider by implementing provider interface