✨ add servicePrincipal methods AddTokenSigningCertificate and SetPref… #151
Conversation
|
@manicminer This should be the base for the terraform provider extension. Please review Tested this in my developer account and can see the thumbprint set on the service principal. |
dhohengassner
force-pushed
the
token-signing-cert
branch
2 times, most recently
from
ce5377a to
ff4dcdd
Compare
|
@manicminer I tried to figure out why the tests are failing but I think this is not related to my changes. It seems all unit tests are failing with the same protocol failure. Locally they seem to work for me. I see the same error in other PRs as well. Is this already known? Any idea is much appreciated. Thanks! |
|
Hi @dhohengassner, it looks like PRs from forks are not able to use OIDC auth. GitHub doesn't have any security model in place for allowing this either, a detail I seem to have skipped over when switching the tests to use this auth method. Until recently we used a private self hosted runner to run the test workflows, alas this is offline for the moment so we'll have to resort to running them manually. You are welcome to post a screenshot of local test results. I will also happily run them when reviewing. |
dhohengassner
force-pushed
the
token-signing-cert
branch
2 times, most recently
from
041a362 to
8395e58
Compare
…erredTokenSigningKeyThumbprint This commit adds support to create the certificiate for Azure AD signed certs and set the preferred token thumbprint on the service principal. This will allow to follow the steps described in https://docs.microsoft.com/en-us/graph/application-saml-sso-configure-api#create-a-signing-certificate using hamilton SDK. Currently Microsoft does not support a method to remove the created certificate key from the service principal. https://docs.microsoft.com/en-us/graph/api/serviceprincipal-addtokensigningcertificate Also manual removal via `removePassword` and `removekey` API calls are not supported and fail with an internal server error. This SDK extension is the base to extend the `terraform-provider-azuread`. Issue: hashicorp/terraform-provider-azuread#732
dhohengassner
force-pushed
the
token-signing-cert
branch
from
8395e58 to
f7a9113
Compare
|
@manicminer think this PR is ready for review Looking forward to your review. Thanks |
There was a problem hiding this comment.
Hi @dhohengassner, thanks again for this contribution and your patience. I've been away for a week so apologies for the delay in reviewing.
Overall this looks great, I just wonder if the ServicePrincipalsClient.SetPreferredTokenSigningKeyThumbprint is needed? Open to your discretion if you think it's important to implement.
Also, if you merge/rebase from main, the tests should now be fixed as the self-hosted runner is back online.
| } | ||
|
|
||
| // SetPreferredTokenSigningKeyThumbprint sets the field preferredTokenSigningKeyThumbprint for a Service Principal. | ||
| func (c *ServicePrincipalsClient) SetPreferredTokenSigningKeyThumbprint(ctx context.Context, servicePrincipalId string, thumbprint string) (int, error) { |
There was a problem hiding this comment.
Whilst I can see the utility here, would you say this method is necessary if this can be done with ServicePrincipalsClient.Update?
|
@dhohengassner I had to make a small change to fix the test but was unfortunately unable to push to your fork. I've opened #158 to replace this PR, preserving your original commit. Many thanks again for your work on this! |
✨ add servicePrincipal methods AddTokenSigningCertificate and SetPreferredTokenSigningKeyThumbprint
This commit adds support to create the certificiate for Azure AD signed certs and set
the preferred token thumbprint on the service principal.
This will allow to follow the steps described in
https://docs.microsoft.com/en-us/graph/application-saml-sso-configure-api#create-a-signing-certificate
using hamilton SDK.
Currently Microsoft does not support a method to remove the created certificate key from the service principal.
https://docs.microsoft.com/en-us/graph/api/serviceprincipal-addtokensigningcertificate
Also manual removal via
removePasswordandremovekeyAPI calls are not supported and fail with an internal server error.This SDK extension is the base to extend the
terraform-provider-azuread.Issue: hashicorp/terraform-provider-azuread#732