Skip to content
This repository has been archived by the owner on Jan 7, 2019. It is now read-only.

Add basic authenticating APIs with register and login #4

Merged
merged 2 commits into from
Nov 12, 2017

Conversation

ltribolet
Copy link
Owner

This adds basic routing to authenticating + a dummy endpoint to verify that everything is working as expected.

Todo : send confirmation mail before really creating a user.

@ltribolet ltribolet self-assigned this Nov 12, 2017
[
'name' => 'required',
'email' => 'required|email',
'password' => 'required',
Copy link
Collaborator

Choose a reason for hiding this comment

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

No pattern ? min/max length, lower/upper ? number etc. ?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Laravel match automatically with Model and no need pattern for email as the validator takes charge of it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

gbkh3cj - imgur

Dumb question why email has 'required|email ?

Copy link
Owner Author

Choose a reason for hiding this comment

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

To validate against an email pattern actually 😛

);

if ($validator->fails()) {
return response()->json(['error' => $validator->errors()], 401);
Copy link
Collaborator

Choose a reason for hiding this comment

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

errors ?

return response()->json(['errors' => $validator->errors()], 401);

Copy link
Owner Author

Choose a reason for hiding this comment

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

What response structure would you like ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

{
  'errors' : '404 error',
  'payload': { 'id': 'zrt' } 
}

payload and errors can be an object, array, array of objects, a string w/e

$modifiedInput['password'] = bcrypt($input['password']);
$user = User::create($modifiedInput);

$request = Request::create('/api/login', 'POST', $input);
Copy link
Collaborator

Choose a reason for hiding this comment

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

'/api/login' shoud be a constant

$response = App::handle($request);

if ($response->getStatusCode() >= 400) {

Copy link
Collaborator

Choose a reason for hiding this comment

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

remove this empty line ?

if ($response->getStatusCode() >= 400) {

throw new HttpResponseException($response);

Copy link
Collaborator

Choose a reason for hiding this comment

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

remove this empty line ?

if ($response->getStatusCode() >= 400) {

throw new HttpResponseException($response);

Copy link
Collaborator

Choose a reason for hiding this comment

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

remove this empty line ?

routes/api.php Outdated

Route::group(['middleware' => 'auth:api'], function(){
Route::post('/logout', 'Api\Auth\LoginController@logout');
ROute::get('/user', 'Api\UserController@index');
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use constant for route URI ?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Apparently this is not the way things are done in Laravel world but that would make sense to me.

@@ -37,6 +37,8 @@ docker-compose exec fpm bash
composer install # this is inside container
cp .env.example .env
php artisan key:generate # copy and paste this key in the APP_KEY section of the .env file
php artisan migrate
php artisan passport:install # copy credentials into .env at PERSONAL_CLIENT_SECRET and PASSWORD_CLIENT_SECRET
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note for the futur automate this in the container ?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yeah ideally I would like to make an installer in the future (graphic one) that would make all of this to avoid any errors from PEBKAC

-H 'postman-token: 091fabe4-1f4f-149d-afb8-ee03b97e23f6' \
-d '{
"email": "mail@domain.co",
"password": "1234",
Copy link
Collaborator

Choose a reason for hiding this comment

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

🙀 😄

Copy link
Owner Author

Choose a reason for hiding this comment

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

security first 😛

# API

API authentication is based on OAuth2 protocol. You'll have to send username/email and password against a token that will used later.
Token has 10 minutes of lifetime and refreshToken 30 days of lifetime.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not hours/days of lifetime. Is it a good practice to refresh the access_token regularly ?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Apparently this a good practice to refresh the token every 10 minutes.

Copy link
Collaborator

@sanghin sanghin Nov 12, 2017

Choose a reason for hiding this comment

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

Alright i didn't know that thx

@sanghin
Copy link
Collaborator

sanghin commented Nov 12, 2017

You should add unit/integration test, time to learn Behat (maybe) 😛

@ltribolet
Copy link
Owner Author

For behavioral testing, I don't feel like to do it right so early stage.

@sanghin sanghin merged commit 6381cd2 into master Nov 12, 2017
@sanghin sanghin deleted the feature/oauth2 branch November 12, 2017 22:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants