Skip to content

Conversation

@despatates
Copy link
Contributor

πŸ”— Linked issue

#411

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

When using method signIn(credentials, { callbackUrl: '/' }) method with local or refresh provider, the callbackUrl and other options were sent to the server.
As mentioned in #411 (comment), some servers can reject the request if the payload contains invalid parameters.

Those options are only used when the sign-in request is done, they shouldn't be sent to the server. They can be set directly in credentials data if they have to.

Flagged as breaking change because some options may have to be copied from signInOptions to credentials.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have added tests (if possible).
  • I have updated the documentation accordingly.

@zoey-kaiser zoey-kaiser added the bug A bug that needs to be resolved label May 22, 2024
@phoenix-ru
Copy link
Collaborator

Hi, thank you for your PR, it looks good already. As per

Flagged as breaking change because some options may have to be copied from signInOptions to credentials.

I think it's safe to assume that options shouldn't be sent to the server and anyone relying on that should instead send options in the sign in body.

@phoenix-ru phoenix-ru merged commit f2f3130 into sidebase:main May 23, 2024
@Ulrich-Mbouna
Copy link

Yes i faced the same issue

@sandrinejoy
Copy link

is this fixed?
I am still facing this issue when using signIn(payload, { external: true, redirect: false });
the options are send as the payload

@despatates
Copy link
Contributor Author

The issue is fixed but not yet released.
If you don't want to wait, install the 0.8.0-alpha.2 version (this is what I did).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug A bug that needs to be resolved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants