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

Omit detailedMessage from login errors #3136

Merged
merged 5 commits into from
Dec 13, 2023

Conversation

jinapurapu
Copy link
Contributor

@jinapurapu jinapurapu commented Nov 28, 2023

Avoids passing detailed error to frontend login errors, no longer differentiable between existing/non-existing Users.

Existing user, wrong password:
Screenshot 2023-12-05 at 2 32 37 PM

User doesn't exist:

Screenshot 2023-12-05 at 2 32 22 PM

Part of https://github.com/miniohq/engineering/issues/1306

@prakashsvmx
Copy link
Member

@jinapurapu , could you check the gofumpt as CI fails ?

@jinapurapu jinapurapu added the WIP This PR is WIP and cannot be merged yet label Nov 28, 2023
@jinapurapu jinapurapu force-pushed the login_error_message_obfuscation branch from f2ddbb2 to f3987ec Compare November 29, 2023 20:19
@jinapurapu jinapurapu removed the WIP This PR is WIP and cannot be merged yet label Nov 29, 2023
@prakashsvmx
Copy link
Member

@jinapurapu there are some test failures , may be related to the messages

@jinapurapu jinapurapu force-pushed the login_error_message_obfuscation branch from 763a205 to aa054bc Compare December 5, 2023 21:07
@jinapurapu jinapurapu changed the title Return generic error for Unauthorized internal ILM credential login attempts Omit detailedMessage from login errors Dec 5, 2023
@bexsoft
Copy link
Collaborator

bexsoft commented Dec 6, 2023

@jinapurapu Please change only the messages in Login APIs, the errors from Object browser are required as there are required to handle a specific corner case

@jinapurapu jinapurapu force-pushed the login_error_message_obfuscation branch from ac47011 to 5dca280 Compare December 6, 2023 21:01
…stinguishable between valid and invalid accessKeys

Cleanup

Cleanup

Gofumpt

Removed detailed errors

Debugging sso test

Remove detailedMessage from login related errors passed to Console

Replaced user_loin with master

Updated tests to reflect removal of detailedMessage for login errors

Removed detailedMessage check from SSO badLogin test

Replaced detailed messsage for LoginNotAllowed

Replaced detailed message for accessdenied error

removed commented lines
@jinapurapu jinapurapu force-pushed the login_error_message_obfuscation branch from 8244644 to 41bd4b5 Compare December 6, 2023 21:10
Copy link
Collaborator

@bexsoft bexsoft left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@cesnietor cesnietor left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the comments, LGTM

@bexsoft bexsoft merged commit 5bc0e74 into minio:master Dec 13, 2023
28 of 30 checks passed
cesnietor pushed a commit to cesnietor/console that referenced this pull request Jan 12, 2024
cesnietor pushed a commit to cesnietor/console that referenced this pull request Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants