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

[bug] missing $fillable property 'secret' #35

Closed
felixsc opened this issue Jan 10, 2021 · 4 comments
Closed

[bug] missing $fillable property 'secret' #35

felixsc opened this issue Jan 10, 2021 · 4 comments
Labels
bug Something isn't working

Comments

@felixsc
Copy link

felixsc commented Jan 10, 2021

Describe the bug
The ConnectedAccount model does not include the secret property in its $fillable array.
Therefore the token secret is not saved when a social account is connected.

Because the CreateConnectedAccount action specifies

'secret' => $providerUser->tokenSecret ?? null,

I think it was forgotton. If you left it out deliberately consider this issue void.

To Reproduce
Steps to reproduce the behavior:

  1. Connect new account
  2. see secret property of ConnectedAccount is empty, despite being passed

Expected behavior
Token secret should be saved with other connected account properties.

Environment context

  • Socialstream version: v2.0.2
  • Jetstream stack: Livewire
  • Laravel version: v8.21.0

And I want to thank you for this package. It's incredible! I was surprised it didn't make it into the Jetstream core.

@felixsc felixsc added the bug Something isn't working label Jan 10, 2021
@joelbutcher
Copy link
Owner

Hi @felixsc thanks for this!

This is actually by design - think of the 'credentials' aspect of the connected account model as the 'password' attribute(s), much like the User model. To assign values to these attributes, this package utilises Eloquent's forceFill method, to update the model regardless of the guarded attributes the user assigns.

@felixsc
Copy link
Author

felixsc commented Jan 11, 2021

I understand your explanation.

But then shouldn't the secret field be automatically filled when registering an account? For me it saved all the other properties (provider, provider_id, nickname, token, etc.) but not the secret. Only after adding it to the $fillable array it stores it. Which shouldn't be necessary with forceFill, right?

Not sure if I'm not understanding the forceFill correctly, if there's a bug with it or if that's a problem with my code, right now 😅 It's working for me, now that I added it to the $fillable array. I just thought maybe other's might run into this as well.

Also, I don't think it should matter: but I'm trying this with the Twitter adapter only.

@joelbutcher
Copy link
Owner

@felixsc I did a bit of investigating last night - you were right, there was a bug, but there's nothing wrong with forceFill.

When creating a Connected Account instance, it was using the following logic:

$user->connectedAccounts()->create([
    //
]);

This has now been changed to:

ConnectedAccount::forceCreate([
    //
]);

I've tagged a patch release for this (v2.0.3)

@felixsc
Copy link
Author

felixsc commented Jan 11, 2021

Ahh, I see. Great news!

Thank you for taking care of it so quickly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants