search

feathersjs / feathers

The API and real-time application framework
https://feathersjs.com
MIT License
14.97k stars 744 forks source link

oauth auth service still called even if `grant` returns an error #3275

Closed tjenkinson closed 9 months ago

tjenkinson commented 9 months ago

Steps to reproduce

Expected behavior

If grant returns an error like it can here

https://github.com/simov/grant/blob/6e0692dfdd83edbc4ee82629ba0fe8f986d5879d/lib/flow/oauth2.js#L69

then the auth service should not be called and an error should returned.

Actual behavior

The error is not checked so the code continues with profile and the access token missing.

It also looks like the provided strategy expects profile to be there: https://github.com/feathersjs/feathers/blob/c619ab2c57f692c419fee610c269c1502b124852/packages/authentication-oauth/src/strategy.ts#L162

Would it make sense to bail here if payload.error is set?

https://github.com/feathersjs/feathers/blob/c619ab2c57f692c419fee610c269c1502b124852/packages/authentication-oauth/src/service.ts#L117

daffl commented 9 months ago

This looks like a really good catch. I bet this is what the issue in https://github.com/feathersjs/feathers/issues/3266 is also about. What's the best way to try and reproduce this?

tjenkinson commented 9 months ago

Ah yes that looks likely. You can trigger it if you set state: true, and then tweak the state in the url when logging in so that it no longer matches in the callback :)