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

Update modules-known-issues.md #8836

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Update modules-known-issues.md #8836

wants to merge 5 commits into from

Conversation

pete-cleary
Copy link

There are no more known issues for the module:
"Simple OAuth / OAuth 2.0
Issue: The module requires a very specific set of permissions for the folder and the keys to be uploaded. Using Private or non-standard filepaths won't work. It is not possible to change these in LIVE or TEST environment.

Solution: You can try to patch the permission check in the module. The alternative is to use off-site key management tools like Lockr"

The module now works without a workaround.

Closes #

Summary

Doc Page Title - <Enter a one sentence summation of the pertinent changes (including not-yet-completed work) provided by this PR.>

Effect

The following changes are already committed:

Remaining Work and Prerequisites

The following changes still need to be completed:

  • List any outstanding work here

Dependencies and Timing

  • Other prerequisites that must be completed before merging this PR

Release:

  • When ready
  • After date: $DATE

Post Launch

Do not remove - To be completed by the docs team upon merge:

  • Redirect /old-path/ => /new-path/ (if applicable)
  • Include/exclude pages ^ respectively within docs search service provider (if applicable)
  • For Heroes - add a props post to the discussion board.
  • Remove from the project board

There are no more known issues for the module:
"Simple OAuth / OAuth 2.0
Issue: The module requires a very specific set of permissions for the folder and the keys to be uploaded. Using Private or non-standard filepaths won't work. It is not possible to change these in LIVE or TEST environment.

Solution: You can try to patch the permission check in the module. The alternative is to use off-site key management tools like Lockr"

The module now works without a workaround.
@pete-cleary pete-cleary requested a review from a team as a code owner January 23, 2024 17:12
Copy link

⚡ Deployed with Pantheon Decoupled

This build was successfully deployed with Pantheon. You can track the build logs here.

👀 Preview: https://pr-8836-documentation.appa.pantheon.site
🛠️ Manage in Pantheon: https://dashboard.pantheon.io/site/2b30153f-e8b1-4427-b076-6109e704ba5d/overview

@rachelwhitton
Copy link
Member

@ryanshoover can you assign someone from your team to review?

@rachelwhitton rachelwhitton added the Process: Needs SME Issue or PR that won't move forward without subject-matter expert contribution label Apr 23, 2024
@rachelwhitton rachelwhitton self-assigned this Apr 23, 2024
@rachelwhitton rachelwhitton removed the Process: Needs SME Issue or PR that won't move forward without subject-matter expert contribution label Apr 23, 2024
@lcatlett
Copy link
Member

this is still an issue and we should recommend the patches to oauth which been tested / validated by other Pantheon customers: https://github.com/lcatlett/oauth2-server/pulls?q=is%3Apr+is%3Aopen+sort%3Aupdated-desc

Patch links:
PHP8: https://patch-diff.githubusercontent.com/raw/lcatlett/oauth2-server/pull/1.patch
PHP7: https://patch-diff.githubusercontent.com/raw/lcatlett/oauth2-server/pull/2.patch

@rachelwhitton
Copy link
Member

@lcatlett are these patches something you expect to contribute upstream to the main project thephpleague/oauth2-server ?

@stevector
Copy link
Contributor

@rachelwhitton can you update this PR to tell customers that these patches are needed (and how to apply them)?

@rachelwhitton
Copy link
Member

@stevector done! reassigned this one to you for next steps

Copy link

⚡ Deployed with Pantheon Decoupled

This build was successfully deployed with Pantheon. You can track the build logs here.

👀 Preview: https://pr-8836-documentation.appa.pantheon.site
🛠️ Manage in Pantheon: https://dashboard.pantheon.io/site/2b30153f-e8b1-4427-b076-6109e704ba5d/overview


**Solution**: Apply the following patches (which have been tested and validated by Pantheon), based on your site's PHP version, to solve for this issue:
* [Oauth Permission Patch - Required to resolve Pantheon known issues (PHP 8.0+)](https://patch-diff.githubusercontent.com/raw/lcatlett/oauth2-server/pull/1.patch)
* [Oauth Permission Patch - Required to resolve Pantheon known issues (PHP 7.4)](https://patch-diff.githubusercontent.com/raw/lcatlett/oauth2-server/pull/2.patch)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* [Oauth Permission Patch - Required to resolve Pantheon known issues (PHP 7.4)](https://patch-diff.githubusercontent.com/raw/lcatlett/oauth2-server/pull/2.patch)
* [Oauth Permission Patch - Required to resolve Pantheon known issues (PHP 7.4)](https://patch-diff.githubusercontent.com/raw/pantheon-systems/oauth2-server/pull/2.patch)

Copy link
Member

@rachelwhitton rachelwhitton Jul 11, 2024

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Not exactly, I think @lcatlett's diff is bigger because it involves a merge commit too?

We probably need @lcatlett's expertise to explain why these patches are necessary. I don't understand why PHP different PHP versions need different patches.

Copy link
Member

@rachelwhitton rachelwhitton Jul 12, 2024

Choose a reason for hiding this comment

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

Ah gotcha, my mistake I'll go ahead and commit this suggestion but mark the convo unresolved and reassign to you for next steps (defer to you on whether we merge or proceed with additional tech review first)

@stevector stevector assigned rachelwhitton and unassigned stevector Jul 11, 2024
@stevector
Copy link
Contributor

@rachelwhitton can you review?

Copy link

⚡ Deployed with Pantheon Decoupled

This build was successfully deployed with Pantheon. You can track the build logs here.

👀 Preview: https://pr-8836-documentation.appa.pantheon.site
🛠️ Manage in Pantheon: https://dashboard.pantheon.io/site/2b30153f-e8b1-4427-b076-6109e704ba5d/overview

@rachelwhitton rachelwhitton removed their assignment Jul 12, 2024
@rachelwhitton rachelwhitton self-requested a review July 12, 2024 16:28
Copy link

⚡ Deployed with Pantheon Decoupled

This build was successfully deployed with Pantheon. You can track the build logs here.

👀 Preview: https://pr-8836-documentation.appa.pantheon.site
🛠️ Manage in Pantheon: https://dashboard.pantheon.io/site/2b30153f-e8b1-4427-b076-6109e704ba5d/overview

@rachelwhitton rachelwhitton added the Process: Needs SME Issue or PR that won't move forward without subject-matter expert contribution label Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Process: Needs SME Issue or PR that won't move forward without subject-matter expert contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants