Inconsistencies in the handling of misspelled URLs

Bloodysunset commented 1 month ago

Bug description

I've tried to implement a login workflow with the REST API and I encountered some difficulties with the endpoints ending with a trailing /.

A bit of context

The first endpoint is /api/v1/security/login. But for some reason, I added a / at the end (it looked like /api/v1/security/login/) and when I tried the query, it promptly responded with :

{
    "errors": [
        {
            "message": "404 Not Found: The requested URL was not found on the server. If you entered the URL manually please check your spelling and try again.",
            "error_type": "GENERIC_BACKEND_ERROR",
            "level": "error",
            "extra": {
                "issue_codes": [
                    {
                        "code": 1011,
                        "message": "Issue 1011 - Superset encountered an unexpected error."
                    }
                ]
            }
        }
    ]
}

Okay, this is a problem I agree with 100%... It's my fault and the API interrupts the request, telling me I've probably misspelled the URL.

By deleting the unnecessary /, I get my access_token; all's well.

The issue

Assuming the / was a problem, I quickly removed it from the other two endpoints (/api/v1/security/csrf_token/ & /api/v1/security/guest_token/). Then I tried to request these endpoints and got a timeout (from my backend which is configured to TO after 10s).

Seeing as the first endpoint TELLS me that I've made a mistake, it didn't occured to me that it could be a spelling issue... No timeout from Superset, no error, nothing...

So I think my issue is more of an open question than a real problem: why do the second and third endpoints end in / and why do they behave differently from the first?

Thank you for reading through this and I hope you have a wonderful day.

How to reproduce the bug

Correct way to handle spelling issue of endpoint

take the /api/v1/security/login endpoint and add a trailing /
send the request
get a wonderful error response

Wrong way

take either /api/v1/security/csrf_token/ or /api/v1/security/guest_token/ and remove the trailing /
send the request
grow old with your grand-kids waiting for a response

Screenshots/recordings

No response

Superset version

master / latest-dev

Python version

3.9

Node version

16

Browser

Chrome

Additional context

No response

Checklist

[X] I have searched Superset docs and Slack and didn't find a solution to my problem.
[X] I have searched the GitHub issue tracker and didn't find a similar bug report.
[ ] I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.

dosubot[bot] commented 1 month ago

Hey there, @Bloodysunset! I'm here to assist you with any bugs, questions, or contributions you may have regarding the repository. Let's work together to tackle this issue.

The endpoints /api/v1/security/csrf_token/ and /api/v1/security/guest_token/ behave differently from /api/v1/security/login when the trailing / is removed because the implementation of these endpoints in the Superset backend explicitly expects the trailing slash.

In the superset/security/api.py file, the csrf_token and guest_token endpoints are defined with a trailing slash:

@expose("/csrf_token/", methods=("GET",))
def csrf_token(self) -> Response:
    return self.response(200, result=generate_csrf())

@expose("/guest_token/", methods=("POST",))
def guest_token(self) -> Response:
    # Implementation details

These definitions mean that the Flask routing mechanism expects the exact URL with the trailing slash. If the trailing slash is removed, the request does not match the defined route, leading to a timeout as the server does not recognize the endpoint.

In contrast, the login endpoint does not have this strict requirement and can handle URLs without the trailing slash, which is why it does not result in a timeout when the trailing slash is removed ^[1]^[2].

To continue talking to Dosu, mention @dosu.

Bloodysunset commented 1 month ago

I guess that the login endpoint behave differently because it's handled by flask-login?

If so, I get that it's not an issue per se but it's still a bit confusing 🤔

apache / superset