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

Fixed Insufficient permissions ("unauthorized") bug #8004

Conversation

ItamarYuran
Copy link
Contributor

@ItamarYuran ItamarYuran commented Jul 24, 2024

Closes #(Insert issue number closed by this PR)

Change Description

Added an if statement checking whether the errorMessage comes back empty (which it always does) in that case we call the AuthorizationError() with the string "Insufficient Permissions".

Background

While there is no access for reading a file through the web ui client, a weird error jumped saying "Gn" instead of stating that this is an Authorization error due to insufficient permissions.

Bug Fix

  1. Problem - While there is no access for reading a file through the web ui client, a weird error jumped saying "Gn"
  2. Root cause - The extractError func returned an empty string
  3. Solution - checking if the string is empty, if so calling AuthorizationError() with the string "Insufficient Permissions"

Testing Details

The code was tested in an dev environment

@ItamarYuran ItamarYuran added bug Something isn't working area/UI Improvements or additions to UI labels Jul 24, 2024
Copy link

E2E Test Results - DynamoDB Local - Local Block Adapter

13 passed

@ItamarYuran ItamarYuran reopened this Jul 24, 2024
@ItamarYuran ItamarYuran added exclude-changelog PR description should not be included in next release changelog include-changelog PR description should be included in next release changelog and removed include-changelog PR description should be included in next release changelog labels Jul 24, 2024
@ItamarYuran ItamarYuran reopened this Jul 24, 2024
@arielshaqed arielshaqed changed the title Fixed Insufficient permmisions bug Fixed Insufficient permissions ("unauthorized") bug Jul 25, 2024
Copy link
Contributor

@arielshaqed arielshaqed left a comment

Choose a reason for hiding this comment

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

Thanks! Approving as this is clearly better 🥳

Some things I would prefer us to do before you pull (but none is blocking!):

  • Open a server-side issue that headObject requests return no error message. I am not sure we can solve it (and remain a REST API), but at least we will have some record of this! It is certainly out of the scope of this PR.
  • Personally I would prefer "Not Authorized" or "Unauthorized" to "Insufficient Permissions"; maybe ask @talSofer pick a good phrasing?

webui/src/lib/api/index.js Outdated Show resolved Hide resolved
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to "Unauthorized" authorized by Tal

Copy link
Contributor

@arielshaqed arielshaqed left a comment

Choose a reason for hiding this comment

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

StillLGTM :-) thanks!

@ItamarYuran ItamarYuran merged commit 873c59d into master Aug 5, 2024
36 checks passed
@ItamarYuran ItamarYuran deleted the 7756-bug-bad-error-message-in-gui-when-unauthorized-and-trying-to-display-readmemd branch August 5, 2024 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/UI Improvements or additions to UI bug Something isn't working exclude-changelog PR description should not be included in next release changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Bad error message in GUI when unauthorized and trying to display README.md
2 participants