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

Update admin authentification #295

Merged
merged 3 commits into from
Oct 10, 2017
Merged

Update admin authentification #295

merged 3 commits into from
Oct 10, 2017

Conversation

pierre-H
Copy link
Contributor

Add api-platform/admin#51 changes and change the request for compatibility with LexikJWTAuthenticationBundle (tested and works)

Add api-platform/admin#51 changes and change the request for compatibility with LexikJWTAuthenticationBundle
 (tested and works)
headers: new Headers({ 'Content-Type': 'application/json' }),
// the next two lines are compatible with LexikJWTAuthenticationBundle. Change this if you use something else ...
body: `_username=${username}&_password=${password}`,
headers: new Headers({ 'Content-Type': 'application/x-www-form-urlencoded' }),
Copy link
Member

Choose a reason for hiding this comment

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

Why can't we keep json login? It has nothing to do with LexikJWTAuthentication see here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just tried in json and lexik return 401 bad credentials :

curl -X POST \
  http://localhost/api/web/login_check \
  -H 'content-type: application/json' \
  -d '{"_username": "admin", "_password": "admin"} '

Only with this configuration I can login. Maybe it's my config but I only have this :

lexik_jwt_authentication:
    private_key_path: '%jwt_private_key_path%'
    public_key_path:  '%jwt_public_key_path%'
    pass_phrase:      '%jwt_key_pass_phrase%'
    token_ttl:        '%jwt_token_ttl%'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So maybe you're right. I thought that because the beginning of this page says that we assume that the api is secure with jwt (with a link to the doc which explains how to use LexikJWTAuthentication) so I thought that this page has to explain how to use admin with LexikJWTAuthentication but if you think that it's should be general ...

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @pierre-H. We should update the doc of the Lexik bundle to explain how to allow JSON login or keep with form-data. (it would be great if we can use JSON everywhere).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does someone have an idea how to use Lexik bundle with json ?

Choose a reason for hiding this comment

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

@pierre-H : Lexik provides some handlers. You just have to use these handlers with the new JSON authenticator.

Note: The JSON authenticator is available since Symfony 3.3.

In app/config/security.yml, you will write something like this:

security:
# ...
    firewalls:
        login:
            anonymous: true
            json_login:
                check_path: /login
                failure_handler: lexik_jwt_authentication.handler.authentication_failure
                success_handler: lexik_jwt_authentication.handler.authentication_success
# ...

In admin, you can now directly send JSON to the API:

            const request = new Request(`${API_ENTRYPOINT}/login`, {
                body: JSON.stringify({
                    username: params.username,
                    password: params.password,
                }),
                headers: new Headers({
                    'Content-Type': 'application/json',
                }),
                method: 'POST',
            });

            return fetch(request).then( /* ... */);

In my repositories, you can find an example secured API and an example of admin.


const restClient = api => hydraClient(api, fetchHydra);

const apiDocumentationParser = entrypoint => parseHydraDocumentation(entrypoint, { headers: new Headers(fetchHeaders) })
Copy link
Member

@soyuka soyuka Sep 28, 2017

Choose a reason for hiding this comment

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

We really should not need this :p. maybe we can pass headers to the react component?

Copy link
Member

Choose a reason for hiding this comment

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

I would rather keep it simplest so anybody can implement their own login and authentication with api. I use token passed in header to backend

@soyuka soyuka requested a review from mysiar September 28, 2017 17:13
@dunglas
Copy link
Member

dunglas commented Sep 28, 2017

@mauchede would you mind reviewing this PR?

@mauchede
Copy link

@pierre-H: Can you add window.location.replace('/'); in the AUTH_LOGIN action? Without this refresh, the API documentation will be empty.

For example:

return fetch(request)
	.then(response => {
	    if (response.status < 200 || response.status >= 300) {
		throw new Error(response.statusText);
	    }

	    return response.json();
	})
	.then(({ token }) => {
	    window.localStorage.setItem(LOCAL_STORAGE_KEY_TOKEN, token);
	    window.location.replace('/');
	});

@pierre-H
Copy link
Contributor Author

pierre-H commented Oct 9, 2017

@mauchede done

@pierre-H
Copy link
Contributor Author

Thank you @mauchede for your explanation. I change the doc so we use json instead of form data

@Simperfit
Copy link
Contributor

@mauchede is it good for you ?

Copy link

@mauchede mauchede left a comment

Choose a reason for hiding this comment

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

Yes, it is!

@Simperfit
Copy link
Contributor

Thanks @pierre-H

@Simperfit Simperfit merged commit 4e04a4b into api-platform:master Oct 10, 2017
@pierre-H pierre-H deleted the patch-1 branch October 10, 2017 13:44
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

Successfully merging this pull request may close these issues.

None yet

6 participants