Make WordPress Core

Opened 4 years ago

Closed 2 years ago

Last modified 2 years ago

#52617 closed enhancement (fixed)

App Passwords: allow http success and reject URLs with local environment type.

Reported by: peterwilsoncc's profile peterwilsoncc Owned by: johnbillion's profile johnbillion
Milestone: 6.2 Priority: normal
Severity: normal Version: 5.6
Component: Application Passwords Keywords: good-first-bug has-patch has-unit-tests commit add-to-field-guide
Focuses: rest-api Cc:

Description

wp_is_authorize_application_password_request_valid() prevents an authorization request if either the success or reject URLs are over insecure connections (http).

On systems with the environment type set to local it would be helpful if these checks were bypassed in line with other aspects of the app passwords component.

Attachments (1)

52617.diff (1.1 KB) - added by viralsampat 2 years ago.
Hello @peterwilsoncc I have checked this patch and its works for me as well. Here, I have added comment for condition.

Download all attachments as: .zip

Change History (19)

#1 @TimothyBlynJacobs
4 years ago

  • Focuses rest-api added
  • Keywords good-first-bug added
  • Milestone changed from Awaiting Review to Future Release
  • Version set to 5.6

#2 @wppunk
4 years ago

Leave it with me. I'm going to fix it.

This ticket was mentioned in PR #1109 on WordPress/wordpress-develop by wppunk.


4 years ago
#3

  • Keywords has-patch added

I've update the wp_is_authorize_application_password_request_valid function and allow to use unsecure connection for the local environment.

Trac ticket: https://core.trac.wordpress.org/ticket/52617

#4 @cadic
3 years ago

I've tested this patch and it works.

With the local environment, it's now possible to authorize an application where a Success URL, Rejection URL or both are server over an insecure connection.

With other environments, the usual "The Authorize Application request is not allowed." error is displayed.

#5 @JeffPaul
2 years ago

@peterwilsoncc probably too late for 6.1, but we could milestone to 6.2 and mark for commit as well perhaps?

#6 @peterwilsoncc
2 years ago

  • Keywords commit added
  • Milestone changed from Future Release to 6.2

probably too late for 6.1, but we could milestone to 6.2 and mark for commit as well perhaps?

Works for me.

@viralsampat
2 years ago

Hello @peterwilsoncc I have checked this patch and its works for me as well. Here, I have added comment for condition.

@peterwilsoncc commented on PR #1109:


2 years ago
#8

G'day Max,

Closing this in favour of PR https://github.com/WordPress/wordpress-develop/pull/3912 as the repo structure has changed a bit and I didn't want to push to your branch.

You'll still get props though.

#9 @peterwilsoncc
2 years ago

  • Keywords needs-unit-tests added; commit removed

I've refreshed the original PR against trunk and it all seems to work via manual testing.

Prior to commit, I'll add some tests to ensure the insecure values are permitted for local environments.

#10 @peterwilsoncc
2 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

Extended the unit tests to include the environment in the linked pull request.

if someone else can sign off of the tests, I think this is good for commit.

Props @costdev for rubber ducking the tests with me.

This ticket was mentioned in Slack in #core by costdev. View the logs.


2 years ago

This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.


2 years ago

#13 @costdev
2 years ago

  • Keywords commit added

@mukesh27 has reviewed and approved PR 3912. I think this is ready for commit consideration now.

This ticket was mentioned in Slack in #core by costdev. View the logs.


2 years ago

#15 @johnbillion
2 years ago

  • Owner set to johnbillion
  • Status changed from new to accepted

#16 @johnbillion
2 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 55283:

Application Passwords: Allow plain HTTP success and reject URLs when using a local environment type.

It's not uncommon for local environments to run over HTTP due to the relative complexity of configuring HTTPS for a local environment. This change allows HTTP URLs for application password responses when that is the case.

Props peterwilsoncc, wppunk, cadic, viralsampat

Fixes #52617

#18 @milana_cap
2 years ago

  • Keywords add-to-field-guide added
Note: See TracTickets for help on using tickets.