NAS-133624 / 25.10 / Support for single time user passwords in STIG mode #11427
Conversation
bugclerk
changed the title
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #11427 +/- ##
==========================================
+ Coverage 83.22% 83.29% +0.06%
==========================================
Files 1673 1676 +3
Lines 59987 60116 +129
Branches 6308 6323 +15
==========================================
+ Hits 49927 50073 +146
+ Misses 10060 10043 -17 ☔ View full report in Codecov by Sentry. |
| @@ -70,4 +82,17 @@ export class TwoFactorGuardService implements CanActivateChild { | |||
| }), | |||
| ); | |||
| } | |||
|
|
|||
| private showStigFirstLoginDialog(): Observable<boolean> { | |||
There was a problem hiding this comment.
- I would avoid stig in naming here because it's conceivable that this feature may be extended for other scenarios in the future.
- How about unifying the flow so that
FirstLoginDialogComponentis always shown when it's either otpw user or 2FA setup is needed? And then lettingFirstLoginDialogComponentdecide which steps to show.
There was a problem hiding this comment.
@undsoft
We have such modal as well, if user missed 2fa configuration:
And then navigated to the 2fa setup page.
There was a problem hiding this comment.
@undsoft - for STIG user, it's impossible to login without one time password or 2fa.
So if I changed password but didn't set up 2fa, I'll see the following.
There was a problem hiding this comment.
So I've updated this dialog to work for both:
OtpwUser and NonOtpwUser, but it's useless at this point.
There was a problem hiding this comment.
What I mean is that we can get rid of showTwoFactorWarning() and only have one branch. If this is an OTPW user OR if 2FA setup is required for normal user, take them to FirstLoginDialogComponent.
If this is an OTPW user, show password form, if this is a normal user, only show the 2FA part.
There was a problem hiding this comment.
Okay, got it, probably I need to add more descriptions there as well.
There was a problem hiding this comment.
Can we request it as a separate ticket? I think it worth a separate ticket since a lot of changes were made already as for this one ticket.
There was a problem hiding this comment.
It would make your code simpler, but okay, please create a ticket then.
...ayout/topbar/change-password-dialog/change-password-form/change-password-form.component.html
Outdated
Show resolved
Hide resolved
...tials/users/one-time-password-created-dialog/one-time-password-created-dialog.component.html
Show resolved
Hide resolved
...entials/users/one-time-password-created-dialog/one-time-password-created-dialog.component.ts
Outdated
Show resolved
Hide resolved
src/app/modules/auth/auth.service.ts
Outdated
| @@ -39,6 +39,7 @@ import { adminUiInitialized } from 'app/store/admin-panel/admin.actions'; | |||
| export class AuthService { | |||
| @LocalStorage() private token: string | undefined | null; | |||
| protected loggedInUser$ = new BehaviorSubject<LoggedInUser | null>(null); | |||
| isOptwPasswordChanged$ = new BehaviorSubject<boolean>(false); | |||
There was a problem hiding this comment.
It's kinda weird to have this here so that two other pieces of code can talk to each other.
See if it's possible to inject TwoFactorGuardService in FirstLoginDialogComponent directly and have that property there directly.
There was a problem hiding this comment.
@undsoft I tried to move it to the TwoFactorGuardService but faced issues.
The main issue is that we should reset that value if user logs out, because if it's a new login with OTPW - the full screen dialog will not show up since password is considered as already changed.
src/app/pages/credentials/users/stig-first-login-dialog/stig-first-login-dialog.component.ts
Outdated
Show resolved
Hide resolved
src/app/pages/credentials/users/user-form/user-form.component.html
Outdated
Show resolved
Hide resolved
|
@undsoft - use latest middleware to make sure it all works. |
AlexKarpov98
requested review from
bvasilenko and
undsoft
and removed request for
a team
|
JIRA ticket https://ixsystems.atlassian.net/browse/NAS-133624 is targeted to the following versions which have not received their corresponding PRs: 25.04-RC.1 |
|
This PR has been merged and conversations have been locked. |
|
backport |
See ticket.
Result:
NAS-133624_1.mov
NAS-133624-2.mov