TheNetworg / oauth2-azure

Azure AD provider for the OAuth 2.0 Client.
https://packagist.org/packages/thenetworg/oauth2-azure
MIT License
230 stars 108 forks source link

Certificate instead of clientSecret #115

Open nowrap opened 4 years ago

nowrap commented 4 years ago

Hello, i want to know if it's possible to use a certifacte instead of a clientSecret?

Regards .nowrap

hajekj commented 4 years ago

I think it should be possible: https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-oauth2-client-creds-grant-flow#second-case-access-token-request-with-a-certificate

It would be cool to actually create a helper method to generate the JWT for client_credentials....

Where would you have your certificate stored? Are we talking a file? Or pulling it from the certificate stores?

nowrap commented 4 years ago

Hello hajekj, thx for your answer.

I guess a certificate as a file or a path would be the easiest approach. Certificate stores i only know from java or mac os. On linux i have seen almost only files (e.g. let's encrypt).

Regards .nowrap

hajekj commented 4 years ago

Is there any specific use-case where would you prefer to use the certificate rather than regular secret? I mean, I would consider a secret in an environment variable about as secure as a file on a filesystem, but I may be wrong. I can understand that it's more secure in certificate store or I could most definitely see a very nice use-case with Azure Key Vault, so that the certificate is never exposed to the server itself.

nowrap commented 4 years ago

The use case are company rules only allowing access via certificates. And we are running on own hardware so the Azure Key Vault is not an option, if i am right?

hajekj commented 4 years ago

Yup, you are right. So I think the best way to start would be to put a code together to create the JWT payload and header as per Microsoft's docs and getting it signed via the certificate (https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-certificate-credentials). I would probably expose it as a method - createAssertionWithCertificate($pathToCertificate) which would return the JWT and then passing the client_assertion_type with value urn:ietf:params:oauth:client-assertion-type:jwt-bearer and client_assertion with value of the generated JWT.

If you ommit the client secret, I believe the library should work, depends on what AAD would do, since even if not specified, the client_secret will be added to the URL even as ...&client_secret=&... as per getAccessToken which might need to get overrided. Unfortunately, I am busy throughout the week to play with it, but I might have some spare time during the weekend to test it out.

nowrap commented 4 years ago

Thx alot!

I started some research for alternative solutions based on linux. The main diffcultiy via PHP is the needed read access to everything to use. So somewhere must be a starting point, like passphrase, databases access, env var, ...

I found this so far: https://www.codementor.io/@ccornutt/keeping-credentials-secure-in-php-kvcbrk55z

uncaught commented 2 years ago

I am interest in this, too, because secrets are only valid for a maximum of 2 years on Azure. A certificate doesn't seem to have a time limit.

uncaught commented 1 year ago

@hajekj could you check the MR? That worked great in my app.

I will create a fork for https://github.com/knpuniversity/oauth2-client-bundle, too, to get those new options in.

hajekj commented 1 year ago

Hello, sorry for late reply, I have reviewed the PR and merged it! Awesome work, thank you so much for it! @uncaught

uncaught commented 1 year ago

@hajekj thank you for the merge!

If you have time, could you publish a new release? Then I can reference the required version of this library in knpuniversity/oauth2-client-bundle for certificates to work :)

And you can close this issue, too, with the release :)

Erwane commented 1 year ago

Hi, with master branch and certificate authentication, i still have AADSTS700023: Client assertion audience claim does not match Realm issuer.. I've fixed my tenant id , double check certificate fingerprint, JWT header and payload, everything looks good and match documentation, but error persist :(

payload

[
'aud' => 'https://login.microsoftonline.com/3d81913c-0473-498f-8829-61a0f999932d/oauth2/v2.0/token',
'exp' => (int) 1684513594,
'iat' => (int) 1684513234,
'iss' => '8b5779b7-7443-4938-8ced-f6b062dd2f11',
'jti' => 'e0e48d69b94cc6f0b627d2eff2253637af11fdaa',
'nbf' => (int) 1684513234,
'sub' => '8b5779b7-7443-4938-8ced-f6b062dd2f11',
]

Header

[
'x5t' => 'ViGYIlTgwCCy4vg9uKawmNytYb8=',
]
uncaught commented 1 year ago

@Erwane the tenant must match your app settings. See these options you have when creating an app on Azure:

image

For each of those options above, you have to set these as tenant:

You can find these values here: https://learn.microsoft.com/en-us/azure/active-directory/develop/v2-oauth2-auth-code-flow#request-an-authorization-code

Erwane commented 1 year ago

My app is created with "Single tenant" option and i set my tenant id exactly how doc say.

This app work well with the "secret key" authentication, not with the certificate.

uncaught commented 1 year ago

Hmm, strange. We've been using this implementation for more than half a year now with no problems. Both with a single tenant and multi tenant.

We are using a few commits before HEAD though - although from the changes that happened since then, I can't see a reason why it would break. But if you like, you could try with "thenetworg/oauth2-azure": "dev-master#dc095e5a6ae485be8a0c8b88a0d07616c18d484b".

Are you using knpuniversity/oauth2-client-bundle, too? I see we configured the end point version there:

knpu_oauth2_client:
  clients:
    azure:
      # ...
      default_end_point_version: '2.0'
      type: azure