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

Unable to delete user #3057

Open
FairyTail2000 opened this issue Sep 21, 2023 · 4 comments · May be fixed by #3352
Open

Unable to delete user #3057

FairyTail2000 opened this issue Sep 21, 2023 · 4 comments · May be fixed by #3352
Assignees
Labels
bug this needs to be fixed community

Comments

@FairyTail2000
Copy link

FairyTail2000 commented Sep 21, 2023

I have created a user with the named "4,^AFzQ&w_6H"D?;_D</,Wd($mkgef^[VRm%o$g-fYUXBV^`98v+#({sQRw?jhg.Ue*H#vs9nwB&"~nHrkJ7q`N#&7!?8Eu4:4!x~joSGmoL%y=oQ}U@/"yz3{g4" which is a kinda dump name I realized after creating it

Expected Behavior

I should be able to delete it from the frontend

Current Behavior

The frontend responds with "not found"
image

Steps to Reproduce (for bugs)

  1. Create a user with given name
  2. Try to delete it
  3. Profit

Regression

No

Your Environment

minio version RELEASE.2023-09-16T01-01-47Z (commit-id=b733e6e83cee28820e815787b0562d17b14f9759)
Runtime: go1.21.1 linux/amd64

  • Server setup and configuration: Not applicable
  • Operating System and version (uname -a): Linux <> 5.10.0-21-cloud-amd64 SMP Debian 5.10.162-1 (2023-01-21) x86_64 GNU/Linux
@harshavardhana harshavardhana transferred this issue from minio/minio Sep 21, 2023
@FairyTail2000
Copy link
Author

Filesystem:
image

@cesnietor
Copy link
Collaborator

We support utf-8 names all characters so we should continue to allow these type of user names. We'll fix this in Console for right path decoding.

@cesnietor cesnietor added bug this needs to be fixed and removed triage labels Sep 21, 2023
@FairyTail2000
Copy link
Author

It looks like the / characters inside is breaking the path building, but I can be wrong

@ramondeklein
Copy link
Contributor

ramondeklein commented May 10, 2024

The actual problem is that base64 encoding is used to encode the username, but standard base64 encoding is not URL-safe (standard base64 encoding uses the + and / characters). To use URL-safe encoding, you should use base64.URLEncoding instead of base64.StdEncoding (RFC 4648). It's almost the same, but instead of the +/ characters, the URL-safe version uses -_.

However, it's not needed to encode the parameters, because Javascript already provides the encodeURIComponent(...) function to encode text. The advantage is that the Go server automatically decodes the parameters without needing to call any functions. Another advantage is that URLs look better (especially if they don't have any encoded characters).

It's a bit strange that the Swagger code generator doesn't perform this encoding, because it's the only sane thing to do. I'm not the only one who's wondering why that's not the default. Fortunately, it can be fixed with a hook that will automatically add this:

module.exports = {
    hooks: {
        onInsertPathParam: (paramName) => `encodeURIComponent(${paramName})`,
    }
};

That way, we don't need to bother which parameters need to be encoded and who don't need it. If they are passed on the URL (via this API module), then encoding is done automatically. If values are passed in the HTTP body (encoded as JSON), then no encoding is required. I have tested this method and it works fine. It does require to refactor all code and to remove the calls to encodeURLString and decodeURLString (unless in places where URLs are created in code).

To fix the issue for now I fixed the encodeURLString and decodeURLString methods to use URL-safe base64 encoding and also fixing the Go server. This fixes the issue and chance of breaking functionality is relative low.

I guess we need to go for the proper option in the long-term to reduce the chances of coding issues. This simplifies code and improves code quality and reliability (in the long-term).

@ramondeklein ramondeklein linked a pull request May 10, 2024 that will close this issue
@ramondeklein ramondeklein linked a pull request May 21, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug this needs to be fixed community
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants