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

API resources not authorized to be accessed on page load cause 403 errors, and are missing filters unless the browser page is reloaded once authorized #1055

Open
andytson-inviqa opened this issue Mar 19, 2019 · 8 comments

Comments

@andytson-inviqa
Copy link

andytson-inviqa commented Mar 19, 2019

I'm putting this issue here because it highlights an entanglement issue of both the api and api-doc-parser, and it's effects on the admin, which also isn't highlighted in the documentation. So the solution may be in more than one repository

To my understanding, api-doc-parser fetches resource query properties from each resource collection endpoint, rather than from the api docs because docs.jsonld doesn't provide these as part of the jsonld spec /docs.jsonld unlike the openapi spec /docs.json.

HydraAdmin then uses the query properties to generate Filters for Lists.

If https://api-platform.com/docs/core/security/ is followed, and an api endpoint is limited to require ROLE_USER, api-doc-parser will attempt to still fetch the properties from every resource's collection get operation prior to react-admin being loaded (or error otherwise if there is no collection get operation), causing a 403 response and not attempt to fetch the properties again within the same browser pageload.

If the resource is later accessed in the same pageload once the user is authorized to use the collection get operation (e.g. from a login that doesn't require the page to reload, or permissions are changed), the List's Filters are missing.

The visual effects of this issue may not be present if react-admin is used directly fully instead as Filters manually defined in react-admin don't appear to rely on the api-doc-parser.

This issue would be present on both basic authentication check (ROLE_USER), and a custom authorization check (any other access control expression), so it isn't solvable by redirecting the user to login.

Its not really feasible for the collection get endpoint to be accessible to all users, nor good for the endpoint to be limited in it's query to return empty dataset in order for the properties to be loaded for HydraAdmin. (In fact some of the endpoints I use are Symfony Messenger-based rather than a data provider, and so shouldn't really have a collection get endpoint anyway.)

This leads me to think that possible solutions for this should be either:
a) The property information is made available by the api on OPTIONS method, and api-doc-parser is set to only perform an OPTIONS request, getting the data from that response, and documentation for the api updated to indicate how to allow OPTIONS but not GET
b) api-docs-parser to use the OpenAPI docs.json as well, to get the properties that way instead
c) extending the jsonld spec to add properties there
d) document that a page reload is required on any change to a user's permissions. (This wouldn't get rid of 403 errors however, but is the simplest workaround)

I expect (a) is much more preferable, although I don't know if it's still an issue needing worrying about that old browsers were unable to do much more than GET/POST to be able to rely on OPTIONS succeeding (I presume not).

This could also lead to the dropping of the need for a collection get operation for resources that don't need them.

Is this worth looking at?

@andytson-inviqa
Copy link
Author

andytson-inviqa commented Mar 19, 2019

Looking at CORSListener, this looks difficult with it setting the response.

@bjrbhre
Copy link

bjrbhre commented Mar 19, 2019

@andytson-inviqa I was also looking at the same issue. Until you have a way to filter based on roles, you can at least avoid being redirected on login by removing the check on 403

// in src/authClient.js
import { AUTH_LOGIN, AUTH_LOGOUT, AUTH_ERROR } from 'admin-on-rest';

export default (type, params) => {
    if (type === AUTH_LOGIN) {
        // ...
    }
    if (type === AUTH_LOGOUT) {
        // ...
    }
    if (type === AUTH_ERROR) {
        const status  = params.message.status;
        if (status === 401 || status === 403) {
            localStorage.removeItem('token');
            return Promise.reject();
        }
        return Promise.resolve();
    }
    return Promise.resolve();
};

Replace

if (status === 401 || status === 403)

by

if (status === 401)

You will simply be displayed an error + blank page
Screenshot 2019-03-19 15 41 36

You might also want to have a look at a similar issue "Dynamic context builder for /docs.jsonld endpoint #866"

@andytson-inviqa
Copy link
Author

I'm afraid it doesn't even get to that point, instead the "Unable to retrieve API documentation." error appears before react-admin is loaded

@bjrbhre
Copy link

bjrbhre commented Mar 19, 2019

When you inspect the network requests, can you spot which ones are failing?

@andytson-inviqa
Copy link
Author

Ok I figured it out, my 403 responses weren't returning JSON, so it was more of a parse error.

There is certainly still the issue of resource properties not being loaded ready for HydraAdmin generated Lists so the ticket is still relevant, but I can reword it a bit, and it doesn't block me I think as I use react-admin directly rather than using the auto-generated HydraAdmin.

@andytson-inviqa andytson-inviqa changed the title API resources not authorized to be accessed are fetched by the api-doc-parser causing it to fail API resources not authorized to be accessed on page load cause 403 errors, and are missing filters unless the browser page is reloaded once authorized Mar 20, 2019
@andytson-inviqa
Copy link
Author

andytson-inviqa commented Mar 20, 2019

Ok, a different effect of this issue happens if ALL api resources are not authorized to be accessible (such as prior to login), causing the same issue I saw before, the react-admin Admin component not being rendered, and an error message instead.

This will at least be mitigated for me by being able to define auth resources that don't require authentication.

@bjrbhre
Copy link

bjrbhre commented Mar 20, 2019

@andytson-inviqa I worked on tackling all the little issues to go from getting started to an actual JWT-secured API + Admin deployed on Heroku + AWS/S3. It might help you. See bjrbhre#1

  • API entities: additional requirements for easier creation of API Entities
  • Deployment: config to deploy on Heroku (API) and AWS/S3 (admin)
  • Security: requirements & config to add JWT in local & deployed environment
  • Automation: Makefile to automate install, provision, deploy

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

No branches or pull requests

2 participants