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

[952] doc - Specify lcobucci/jwt version, fix deprecation #953

Merged
merged 4 commits into from
Mar 28, 2021

Conversation

amacrobert-meq
Copy link
Contributor

Fixes #952

doc/security.md Outdated Show resolved Hide resolved
@GrahamCampbell
Copy link
Contributor

Actually, the code should be the following, for compatibility with both 3.4.x and 4.x:

$config = Configuration::forSymmetricSigner(
    new Sha256(),
    LocalFileReference::file('path/to/integration.private-key.pem')
);

$jwt = $config->builder()
    ->issuedBy($integrationId)
    ->issuedAt(time())
    ->expiresAt(time() + 60)
    ->getToken($config->signer(), $config->signingKey()));

$github->authenticate($jwt, null, Github\Client::AUTH_JWT)

@GrahamCampbell
Copy link
Contributor

GrahamCampbell commented Dec 16, 2020

with the following imports at the top:

use Lcobucci\JWT\Configuration;
use Lcobucci\JWT\Signer\Key\LocalFileReference;
use Lcobucci\JWT\Signer\Rsa\Sha256;

in place of the other Lcobucci\JWT imports.

Update "integrating with app" example to use lcobucci/jwt 3.4

Fix whitespace formatting

update example for compatibility with both 3.4.x and 4.x
@amacrobert-meq
Copy link
Contributor Author

Closing this PR because I don't know why CI is failing (I did update the head branch with the most recent version of the base) and tbh I've lost interest in figuring it out.

@acrobat
Copy link
Collaborator

acrobat commented Mar 18, 2021

@amacrobert-meq Sorry I've lost track of this PR, the failing check can be ignored as it is unrelated. I think the only "action" left here was to implement the sugested changes of @GrahamCampbell or are these already "resolved"? I think these are already applied by you?

So I think we can re-open this PR and have it ready to be merged!

@amacrobert-meq
Copy link
Contributor Author

@acrobat Ok, in that case I'm reopening

Copy link
Collaborator

@acrobat acrobat left a comment

Choose a reason for hiding this comment

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

The PR looks good! I've only added 1 change to avoid triggering deprecations otherwise good to merge!

doc/security.md Outdated Show resolved Hide resolved
…integration-doc-update

* upstream/2.x:
  Correctly link to github actions docs and fix backlinks
  Improved bc check
  bug KnpLabs#979 Deployments: use proper media-type for in_progress/queued, inactive state (staabm)
Copy link
Collaborator

@acrobat acrobat left a comment

Choose a reason for hiding this comment

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

I've pushed the requested changes, all good now.

@acrobat acrobat merged commit d343143 into KnpLabs:2.x Mar 28, 2021
@acrobat
Copy link
Collaborator

acrobat commented Mar 28, 2021

Thanks @amacrobert-meq! And congrats on your first contribution! 🎉

acrobat added a commit that referenced this pull request Mar 28, 2021
* 2.x:
  bug #953 [952] doc - Specify lcobucci/jwt version, fix deprecation (amacrobert-meq, acrobat)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Authenticating as an Integration" doc is out of date
3 participants