mitodl / mit-open

BSD 3-Clause "New" or "Revised" License
0 stars 2 forks source link

API redirecting to URL with a trailing slash if one is not included #445

Open gumaerc opened 8 months ago

gumaerc commented 8 months ago

Expected Behavior

The API returns a response

Current Behavior

The API sends back a 301 permanent redirect to the URL with a trailing slash at the end, which given the example below would be https://mit-open-rc.odl.mit.edu/api/v0/users/me/?format=json

Steps to Reproduce

  1. Navigate to an API endpoint like https://mit-open-rc.odl.mit.edu/api/v0/users/me?format=json

Additional Details

This was discovered while testing https://github.com/mitodl/mit-open/pull/429/. The mit-open API implementation in OCW is entirely done from frontend Javascript, with fetch commands (at least for now). When performing a fetch using a site that has to be accessed with CORS, a redirect breaks the flow. Since 3xx responses are not part of the CORS spec, the response is returned sans CORS headers. Since the request is a CORS request to begin with and the Access-Control-Allow-Origin header is not returned by the server with the 301, the browser generates a CORS error and does not follow the redirect. In order for this strategy to work in OCW, the API URLs need to follow the exact schema the API needs, and / or the API needs to not redirect and enforce a trailing slash.

mbertrand commented 8 months ago

This is default behavior in django apps, which can be overrriden by including APPEND_SLASH=False in environment settings, but without the trailing slash a 404 would be returned instead. So I think the frontend just needs to include the trailing slash in link urls.

mbertrand commented 8 months ago

There are workarounds though if we want the trailing slash to be optional and have the url work with or without it.

gumaerc commented 8 months ago

@mbertrand I see that, and ensuring that we use trailing slashes in the API URLs isn't a problem. When I was talking to @rhysyngsun about this though, he seemed to think that he had made the trailing slash optional already. Since it is not currently optional, should it be? If it was intended for it to function the way it does currently, that's fine. We can simply make sure we add trailing slashes to all the endpoints hit by mit-open-login-button. If the trailing slash was made optional though and this functionality was broken somehow, I'm wondering if we should fix it or not. If not, we can close this issue.

sovsey commented 3 months ago

@gumaerc Can you confirm is this still an issue?

gumaerc commented 3 months ago

@sovsey I can confirm that the issue as described still exists, although the URLs above have changed. For example, if you hit https://api.mitopen-rc.odl.mit.edu/api/v0/users/me?format=json then you are redirected to https://api.mitopen-rc.odl.mit.edu/api/v0/users/me/?format=json.

As to whether or not we actually need to address this, I'm not sure. The mit-open-login-button library I mention is an experimental feature that allows you to embed a control in external websites that first checks if you are logged in by directly hitting the API using cookie auth. If you are not logged in, a button linking to mit-open login is presented. If you are logged in, it displays some text that says "logged in as x" where x is your username.

Further experimentation proved that we can export whole components from mit-open as a separate webpack bundle for import into other projects. I tested this with the UserList modal components, but in theory it could be used with any component, including the UserMenu control in the header or even a separate component for external login if we wanted to create that. I believe using Axios to hit the API, rather than doing it directly with fetch like mit-open-login-button does, should allow us to hit the login endpoint without issue but I'm not 100% certain.

In theory, this issue can be closed as it does exist but is maybe not a problem. We should maybe wait until we do further work on integrating mit-open into OCW though.