CSS-7044 Added mock authenticator #1160
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kian99
requested review from
alesstimec,
ale8k,
babakks and
mina1460
as code owners
alesstimec
approved these changes
Feb 19, 2024
mina1460
reviewed
Feb 20, 2024
| // for user object creation | ||
| // | ||
| // This method is exported for use by the mock authenticator. | ||
| func VerifySessionToken(token string, secretKey string) (jwt.Token, error) { |
There was a problem hiding this comment.
I am not sure i understand why is this a separate function and not directly inside func (as *AuthenticationService) VerifySessionToken
There was a problem hiding this comment.
in other words, why do we need a wrapper and a function, and not only the function
There was a problem hiding this comment.
Good question. As a method it's basically a class method, as a function it's like a static method namespaced to the auth package. It probably shouldn't have been added as a method on the struct, but I can't remove it without also changing the interface (this method implements the OAuth interface). So I'll keep it like this until Alex is back and we can discuss changing the interface.
babakks
approved these changes
Feb 20, 2024
mina1460
approved these changes
Feb 20, 2024
| // for user object creation | ||
| // | ||
| // This method is exported for use by the mock authenticator. | ||
| func VerifySessionToken(token string, secretKey string) (jwt.Token, error) { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adds an empty mock authenticator to the JIMM suite used for tests. Note that the mock is mostly empty except for
VerifySessionToken, this is intentional. Reason being, in most tests we don't need to test the full login flow so only theVerifySessionTokenfunctions needs to be implemented. The test login function can generate a JWT with pre-defined key that is the same as the MockAuthenticator. The removal of Candid will require the Juju login changes to land in Juju, so that we can bump our Juju version and start to use the new login flows.Fixes CSS-7044
Engineering checklist
Check only items that apply