Skip to content

fix(auth): Skip login when using noop auth #328

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Oct 25, 2021

Conversation

jan-law
Copy link
Contributor

@jan-law jan-law commented Oct 20, 2021

Fixes #327

NoopAuthForm automatically calls onSubmit on first load. The user is immediately directed to the dashboard.

@jan-law jan-law added the fix label Oct 20, 2021
@jan-law jan-law requested a review from andrewazores October 20, 2021 17:09
@@ -199,7 +200,13 @@ export class LoginService {
}

private completeAuthMethod(method: string): void {
this.authMethod.next(method);
const validMethod = method as AuthMethod;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can still end up broken :-)

If I modify the Cryostat backend so that the BasicAuthManager reports its authentication method as "Broken", then the string 'Broken' will be passed into this function as method. This happens at runtime, where the as AuthMethod type cast means nothing (since this is only a TypeScript compile-time check). !!validMethod will be true because validMethod is just method which is a non-empty string, and so we call authMethod.next('Broken').

I think instead of a typecast like method as AuthMethod, what you wanted was to try to reverse-map from the string to the enum, like so: https://stackoverflow.com/questions/43804805/check-if-value-exists-in-enum-in-typescript

let validMethod;
if (Object.values(AuthMethod).includes(method)) {
  validMethod = method as AuthMethod;
} else {
  validMethod = AuthMethod.UNKNOWN;
}
this.authMethod.next(validMethod);
this.authMethod.complete();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried moving the typecast before Object.values(AuthMethod).includes to prevent the error below. Let me know what you think.

ERROR in src/app/Shared/Services/Login.service.tsx:204:44
TS2345: Argument of type 'string' is not assignable to parameter of type 'AuthMethod'.
    202 |   private completeAuthMethod(method: string): void {
    203 |     let validMethod;
  > 204 |     if (Object.values(AuthMethod).includes(method)) {
        |                                            ^^^^^^

@andrewazores andrewazores merged commit cd121c5 into cryostatio:main Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NoopAuth should not display a Login form
2 participants