webfox / laravel-xero-oauth2

A Laravel integration for Xero using the Oauth 2.0 spec
MIT License
50 stars 32 forks source link

Request: Store credentials in Laravel's Session #76

Closed chriswait closed 8 months ago

chriswait commented 2 years ago

Hi, big fan of this library which has saved me about a billion hours of work 🙏 thanks

Is your feature request related to a problem? Please describe. I'm building a Laravel app which needs to hit Xero's API using different credentials on behalf of multiple different users (across various tenants/organisations).

I need to store credentials against each user, but the laravel app doesn't authenticate users (yet) so there's no User model to store credentials under (as described here)

Describe the solution you'd like I'd like an OauthCredentialManager implementation that stores credentials in the Laravel Session. This store could be bound in AppServiceProvider, and would use Laravel's session to store both:

  1. oauth "state" for redirects (xero_oauth2_state)
  2. the xero api credentials for the requesting user

Describe alternatives you've considered

Additional context I have an implementation of this which seems to be working locally, and would be very happy to tidy it up and submit a PR if you'd be interested.

Thanks for reading!

chriswait commented 2 years ago

One problem with this approach might be security.

If a developer uses SESSION_DRIVER=cookie and hasn't configured config('session.secure') or config('session.encrypt') appropriately, it's possible the cookie storing the xero credentials could be stolen by an attacker. Depending on the scopes requested this could be pretty bad.

Perhaps the Store class could check for this condition and throw an error? Happy to look into this.

hailwood commented 2 years ago

Hi @chriswait,

Thank you for your kind words :) I definitely appreciate your offer, however I believe that storing the credentials in such a temporary manner is a rather specific usecase so I'll have to decline a PR for it. You're welcome to attach your code to this issue though and submit a PR to update the readme with a note about an example using session storage and I'll merge that :+1:

As a note on your security concerns, perhaps you could instead still store the credentials for each user in the cache, but use 'xero-credentials-' . Session::getId() as the key? That would prevent the credentials being attached to the cookie :)

Cheers, Matt