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.

Sign up for GitHub

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

Jump to bottom

Enabled client_secret_basic authentication on requestClientCredentialsToken() #348

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Enabled client_secret_basic authentication on requestClientCredentialsToken() #348

wants to merge 10 commits into from

Conversation

Magentron
Copy link

@Magentron Magentron commented Nov 28, 2022 •
edited
Loading

Fixes issue #347

List of common tasks a pull request require complete

  • Changelog entry is added or the pull request don't alter library's functionality

@Magentron
Enabled client_secret_basic authentication on requestClientCredentia…
cb6cec4 
…lsToken() jumbojett#347
@Magentron
updated composer.json added replace, updated README.md added notifica…
8ef1281 
…tion about project forked from
@Magentron
Merge commit '102a5bba6c446beceb87d6f57dd337a0d34d12ec' into features…
90b959b 
…/allow-client_secret_basic-on-request-token

* commit '102a5bba6c446beceb87d6f57dd337a0d34d12ec':
  Removed interface and added function that can be extended to add the jwe functionality
  Updated changelog
  Added id token jwe decryption
  set response as jwt when not jwe
  use correct types
  Added userInfo response type check to handle signed and encrypted responses

# Conflicts:
#	CHANGELOG.md
@Magentron
Merge branch 'master' into features/allow-client_secret_basic-on-requ…
5b3bd9a 
…est-token
@Magentron
Merge commit 'e46f108adf9be166f232edfe1e5315e6fcf168e5' from jumbojet…
5e83bc4 
…t/master

* commit 'e46f108adf9be166f232edfe1e5315e6fcf168e5':
  chore: Update visibility of getWellKnownConfigValue to protected (jumbojett#363)
  chore: Update construct typehint in docblock (jumbojett#364)
  Update CHANGELOG.md
  docs: fix changelog format
  Correct variable docstring
  Fix if statement to throw error instead of bypass
  Add an extra check on $_REQUEST['state']
  Fix return type
  Removed interface and added function that can be extended to add the jwe functionality
  Remove unnecessary method overrides in UnitTest
  Add Changelog entry
  Fix LogoutToken verification for single value `aud` claims
  Updated changelog
  Added id token jwe decryption
  set response as jwt when not jwe
  use correct types
  Added userInfo response type check to handle signed and encrypted responses

# Conflicts:
#	CHANGELOG.md
@Magentron
Merge branch 'upstream/master'
41301c6 
* upstream/master:
  fix: Removed duplicate check on jwks_uri and only check if jwks_uri exists when needed (jumbojett#373)
  fix: Check if subject is equal to subject of id token when verifying JWT claims (jumbojett#406)
  fix: Cast SERVER_PORT to integer (jumbojett#404)
  chore(deps): bump actions/cache from 3 to 4 (jumbojett#417)
  chore(deps): bump actions/checkout from 2 to 4 (jumbojett#416)
  docs: Update README.md to correct addScope parameter type in 1.0.0 (jumbojett#405)
  chore: Update ci to support php 8.3 and add dependabot (jumbojett#407)
  release: 1.0.0 (jumbojett#402)
  Set the User-Agent regardless of GET or POST (jumbojett#382)
  fix: Update well known config value function response types (jumbojett#376)
  feat: set useragent (jumbojett#370)
  feat: php7.0 minimum requirement (jumbojett#327)

# Conflicts:
#	CHANGELOG.md
@Magentron
Merge branch 'master' into features/allow-client_secret_basic-on-requ…
c9ee737 
…est-token

* master:
  fix: Removed duplicate check on jwks_uri and only check if jwks_uri exists when needed (jumbojett#373)
  fix: Check if subject is equal to subject of id token when verifying JWT claims (jumbojett#406)
  fix: Cast SERVER_PORT to integer (jumbojett#404)
  chore(deps): bump actions/cache from 3 to 4 (jumbojett#417)
  chore(deps): bump actions/checkout from 2 to 4 (jumbojett#416)
  docs: Update README.md to correct addScope parameter type in 1.0.0 (jumbojett#405)
  chore: Update ci to support php 8.3 and add dependabot (jumbojett#407)
  release: 1.0.0 (jumbojett#402)
  Set the User-Agent regardless of GET or POST (jumbojett#382)
  fix: Update well known config value function response types (jumbojett#376)
  feat: set useragent (jumbojett#370)
  feat: php7.0 minimum requirement (jumbojett#327)
  updated composer.json added replace, updated README.md added notification about project forked from

# Conflicts:
#	CHANGELOG.md
@Magentron
Merge branch 'upstream/master'
9b64e0d 
* upstream/master:
  fix: protected responseContentType to allow overloading of fetchUrl function (jumbojett#446)
  test: unit tests for verifyJWTClaims and different aud claims (jumbojett#443)
  Fix TypeError in `verifyJWTClaims` (jumbojett#442)
  release: v1.0.2 (jumbojett#439)
  test: add unit test for SERVER_PORT type cast (jumbojett#438)
  fix: bring back jumbojett#404 (jumbojett#437)
  release: v1.0.1 (jumbojett#432)
  fix: protected $responseCode to allow proper overloading of fetchURL() (jumbojett#433)
  chore(deps-dev): update yoast/phpunit-polyfills requirement from ^1.0 to ^2.0 (jumbojett#430)
  chore(deps): update phpseclib/phpseclib requirement from ~3.0 to ^3.0.7
  ci: run GitHub workflows on pull requests and pushes to master (jumbojett#431)
  chore: enable dependabot for composer (jumbojett#429)
  fix: handle JWT decode of non JWT tokens (jumbojett#428)
  fix: method signatures after 1.0 release (jumbojett#427)
@Magentron
Merge branch 'master' into features/allow-client_secret_basic-on-requ…
48d38b3 
…est-token

* master:
  fix: protected responseContentType to allow overloading of fetchUrl function (jumbojett#446)
  test: unit tests for verifyJWTClaims and different aud claims (jumbojett#443)
  Fix TypeError in `verifyJWTClaims` (jumbojett#442)
  release: v1.0.2 (jumbojett#439)
  test: add unit test for SERVER_PORT type cast (jumbojett#438)
  fix: bring back jumbojett#404 (jumbojett#437)
  release: v1.0.1 (jumbojett#432)
  fix: protected $responseCode to allow proper overloading of fetchURL() (jumbojett#433)
  chore(deps-dev): update yoast/phpunit-polyfills requirement from ^1.0 to ^2.0 (jumbojett#430)
  chore(deps): update phpseclib/phpseclib requirement from ~3.0 to ^3.0.7
  ci: run GitHub workflows on pull requests and pushes to master (jumbojett#431)
  chore: enable dependabot for composer (jumbojett#429)
  fix: handle JWT decode of non JWT tokens (jumbojett#428)
  fix: method signatures after 1.0 release (jumbojett#427)
DeepDiver1975
DeepDiver1975 requested changes Nov 7, 2024
View reviewed changes
Copy link
Collaborator

@DeepDiver1975 DeepDiver1975 left a comment

Choose a reason for hiding this comment

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

We can only merge this once the traces of the fork are removed again. THX

README.md Outdated
@@ -1,3 +1,5 @@
NB: This is a fork from [jumbojett/OpenID-Connect-PHP](https://github.com/jumbojett/OpenID-Connect-PHP) to allow client basic authentication on obtaining the access token.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please revert this change.

Copy link
Author

Choose a reason for hiding this comment

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

Reverted

README.md Outdated
@@ -14,7 +16,7 @@ A special thanks goes to Justin Richer and Amanda Anganes for their help and sup
## Install ##
1. Install library using composer
```
composer require jumbojett/openid-connect-php
composer require magentron/openid-connect-php
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please revert this change.

Copy link
Author

Choose a reason for hiding this comment

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

Reverted

composer.json
@@ -1,6 +1,6 @@
{
"name": "jumbojett/openid-connect-php",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please revert this change.

Copy link
Author

Choose a reason for hiding this comment

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

Reverted

@Magentron
Copy link
Author

@DeepDiver1975 Should I resolve the requested changes or will you?

@DeepDiver1975
Copy link
Collaborator

Your PR... Your responsibility to get this merged 😁
So yes ... Please fix it. THX

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

@DeepDiver1975 DeepDiver1975 Awaiting requested review from DeepDiver1975

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Magentron @DeepDiver1975