Skip to content
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

[Fix-16668][api-server] Fix SSO login bug #16669

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open

[Fix-16668][api-server] Fix SSO login bug #16669

wants to merge 4 commits into from

Conversation

mzxc
Copy link

@mzxc mzxc commented Sep 27, 2024

Purpose of the pull request

fix #16668

Brief change log

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

Pull Request Notice

Pull Request Notice

If your pull request contain incompatible change, you should also add it to docs/docs/en/guide/upgrede/incompatible.md

mzxc and others added 2 commits September 26, 2024 16:39
when use sso login type, refresh login page and click sso login button,
will dialog a error tips: State or code entered incorrectly.
@SbloodyS SbloodyS added bug Something isn't working first time contributor First-time contributor labels Sep 27, 2024
@SbloodyS SbloodyS added this to the 3.3.0 milestone Sep 27, 2024
@@ -158,7 +158,8 @@ public Result ssoLogin(HttpServletRequest request) {
if (session.getAttribute(Constants.SSO_LOGIN_USER_STATE) == null) {
session.setAttribute(Constants.SSO_LOGIN_USER_STATE, randomState);
}
return Result.success(((AbstractSsoAuthenticator) authenticator).getSignInUrl(randomState));
return Result.success(((AbstractSsoAuthenticator) authenticator)
.getSignInUrl(session.getAttribute(Constants.SSO_LOGIN_USER_STATE).toString()));
Copy link
Member

Choose a reason for hiding this comment

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

LGTM. Let's make it better.

            HttpSession session = request.getSession();
            if (session.getAttribute(Constants.SSO_LOGIN_USER_STATE) == null) {
                String randomState = UUID.randomUUID().toString();
                session.setAttribute(Constants.SSO_LOGIN_USER_STATE, randomState);
            }
            return Result.success(((AbstractSsoAuthenticator) authenticator)
                    .getSignInUrl(session.getAttribute(Constants.SSO_LOGIN_USER_STATE).toString()));

Copy link

sonarcloud bot commented Oct 6, 2024

Please retry analysis of this Pull-Request directly on SonarCloud

Copy link
Contributor

@davidzollo davidzollo left a comment

Choose a reason for hiding this comment

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

+1
LGTM

@davidzollo
Copy link
Contributor

By the way, do not close the related issue when the pr is open. The issue will be automatically closed when the pr is merged.
Thanks for your first contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend bug Something isn't working first time contributor First-time contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] [API] SSO login bug
4 participants