-
Notifications
You must be signed in to change notification settings - Fork 43
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
Update login form display strategy #669
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, I think the behaviour is already like that, isn't it? See https://github.com/indigo-iam/iam/blob/develop/iam-login-service/src/main/webapp/WEB-INF/views/iam/login.jsp#L76.
Say, When Admin has set loginPageMode to Hidden (Might Happen edge case: Users who know the route link will still be able to access the login form even though admin doesn't want users to enter credentials to login). Does that make sense? |
Ok, clear now. Yes, it makes sense to me. |
f192d11
to
2247a1c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-thinking about this issue, the 'HIDDEN case' is already used in production and we don't want to break it. Then, you can add another property, such as DISABLED to reproduce your behavior.
No worries. I thought the DISABLED Case is same as HIDDEN. If it is make no sense. I am happy to close this PR :) |
My only comment is that I'd change the "title" of this PR/fix. We're not preventing access, we're changing (in a correct way) the logic that hides a form. The login endpoint still login you if you present your right credentials (through a curl e.g.). Then, I'll update the PR title in order to be more clear about this. Something like "Update login form display strategy" e.g. |
Kudos, SonarCloud Quality Gate passed! |
LGTM |
Quality Gate passedIssues Measures |
Hi @Sae126V, we were reviewing your PR that is fine.
The local authentication is still shown by adding Let's decide together which behavior is preferred. |
Prevents access to the login form.
Need to prevent access to the login form when admin has decided to disable(Set to false) both
localAuthenticationVisible
andshowLinkToLocalAuthn
.