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

Finish implementing OAuth & test it #10

Closed
RossComputerGuy opened this issue Feb 28, 2021 · 10 comments
Closed

Finish implementing OAuth & test it #10

RossComputerGuy opened this issue Feb 28, 2021 · 10 comments
Assignees
Labels
enhancement New feature or request

Comments

@RossComputerGuy
Copy link
Contributor

Definitely should be unit tests for OAuth and the rest of the OAuth model spec from this webpage should be implemented.

@RossComputerGuy RossComputerGuy added the enhancement New feature or request label Feb 28, 2021
@RossComputerGuy RossComputerGuy self-assigned this Feb 28, 2021
@RossComputerGuy
Copy link
Contributor Author

Right now, only password grant is supported. Would be better having all grants implemented.

@RossComputerGuy
Copy link
Contributor Author

~ > curl -L "http://172.18.0.4:3000/v1/user/token" -X POST -d "grant_type=password&client_id=0&client_secret=0&username=test&password=12345678" | jq           ~
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   152  100    63  100    89    851   1202 --:--:-- --:--:-- --:--:--  2054
{
  "error": "server_error",
  "error_description": "Invalid resource"
}

Another problem, figuring out how to fix some errors are difficult.

server_api_1   | debug: receving request from http://172.18.0.4/v1/user/token (POST)
server_api_1   | debug: Getting client: 0 *
server_api_1   | Executing (default): SELECT `id`, `secret`, `grants`, `createdAt`, `updatedAt` FROM `clients` AS `client` WHERE `client`.`id` = 0 AND `client`.`secret` = '0';
server_api_1   | debug: Getting user: test ********
server_api_1   | Executing (default): SELECT `uuid`, `username`, `password`, `birthdate`, `email`, `createdAt`, `updatedAt` FROM `users` AS `user` WHERE `user`.`username` = 'test';
server_api_1   | Executing (default): INSERT INTO `accessTokens` (`token`,`uuid`,`expires`,`client_id`,`createdAt`,`updatedAt`) VALUES (?,?,?,?,?,?);
server_api_1   | Executing (default): SELECT `id`, `secret`, `grants`, `createdAt`, `updatedAt` FROM `clients` AS `client` WHERE `client`.`id` = '0';
server_api_1   | Executing (default): SELECT `uuid`, `username`, `password`, `birthdate`, `email`, `createdAt`, `updatedAt` FROM `users` AS `user` WHERE `user`.`uuid` = '0'

@andersevenrud
Copy link
Collaborator

Another problem, figuring out how to fix some errors are difficult.

I have a comment about exactly this here: 0e2016a

This needs to be improved. Once database relations are properly set up, you can actually do a join query, which means that you will basically only have one database call which will make this a lot easier to handle and you can dump the errors straight into winston.

The reason I left the error message generic ("invalid resource") is because this is what the end-user will see.

I would have done this myself, but I did not feel comfortable touching the oauth stuff in my rebase PR.

@andersevenrud
Copy link
Collaborator

But you can actually pinpoint the problem from your log there. The last SELECT query probably did not return a result, and you can see where that occured pretty easily from the source:

const tokenUser = await User.findOne({ where: { uuid: client.id } })

@RossComputerGuy
Copy link
Contributor Author

Looks like a typo there.

@RossComputerGuy
Copy link
Contributor Author

I fixed the typo and now OAuth is working, will leave this issue open until all OAuth grants are implemented.

@andersevenrud
Copy link
Collaborator

Also, add tests ;)

@RossComputerGuy
Copy link
Contributor Author

That too

@RossComputerGuy
Copy link
Contributor Author

These grants are currently implemented:

  • authorization_code
  • password

The other standard types supported by oauth2-server still need to be implemented. These grants are:

  • client_credentials
  • implicit
  • refresh_token

RossComputerGuy added a commit that referenced this issue Mar 7, 2021
* api: working on #14

* api: fixed typo

* OAuth is now working

* api: add types for cors

* Removed the secrets

* Use NGINX for CORS

* account: removed console.log
@RossComputerGuy
Copy link
Contributor Author

No longer relevant with how the website functions now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants