Skip to content

Conversation

@gassan
Copy link
Contributor

@gassan gassan commented Jul 10, 2019

If Identity Provider belongs to the same organisation there is no need to redirect end user to open-id server. It must be possible to login with username and password on wordpress site. 'Direct Access Grants aka grant_type=password' have to be enabled for client. This option is disabled by default.

made plugin to singleton. It would be good to have an access to settings without reading and parsing options.

…rganisation there is no need to redirect end user to open-id server. It must be possible to login with username and password on wordpress site. 'Direct Access Grants' have to be enabled for client. This option is disabled by default.

made plugin to singleton. It would be good to have an access to settings without reading and parsing options.
else {
wp_redirect( home_url() );

if ( is_null($user) || is_wp_error($user) && $user->get_error_code() === 'invalid_username' ) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need a little clarity on this check. Do we want to test for the exact message 'invalid_username' here?

Implication being that we only send a remote authentication request if the provided username doesn't exist in WP?

Copy link
Contributor Author

@gassan gassan Jul 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right.
In current implementation (priority 15) makes this check no sense. Its make sense only if this filter runs after the default wordpress authenticator (priority 20). the default returns this WP_Error if a user not found in [prefix_]users table.
Q: What would be better here?

  1. to modify this code and remove this check.
  2. to allow administrator to set this priority on settings page? (default 15)

@timnolte timnolte added the enhancement Issues & PRs related to new features. label Aug 20, 2020
@timnolte timnolte added the status: needs review PR that needs review. label Aug 27, 2020
@timnolte timnolte closed this Jan 20, 2021
@timnolte timnolte added duplicate and removed enhancement Issues & PRs related to new features. status: needs review PR that needs review. labels Jan 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants