Azure / azure-libraries-for-java

Azure Management Libraries for Java
https://docs.microsoft.com/en-us/java/azure/
MIT License
94 stars 97 forks source link

[Bug] Service Principal add or remove password credential fails #1290

Open paulojohnj opened 3 years ago

paulojohnj commented 3 years ago

Currently a service principal update that either adds or removes a password credential fail, complaining about an additionalProperties field.

In addition, the api call that these calls are making appears to be incorrect, making a PATCH request. When adding or removing a password the graph api appears to require a POST call to a differing endpoint (the graph api docs note that the PATCH request is no longer valid for add or remove).

This was validated by using the graph api sdk directly, which was able to make the add/remove request directly.

While in the graph sdk it appears that

yungezz commented 3 years ago

hi @xccc-msft could you pls have a look? thakns

xseeseesee commented 3 years ago

@paulojohnj Would you please share some code snippet and the version of SDK you are using now? Thanks.

paulojohnj commented 3 years ago

SDK version: 1.36.3

Working initial service principal create call:

@Test
    @Ignore
    public void sp() throws InvalidConfigurationException {
        final Azure azure = new AzureCloudProvider(cloud).getAzureClient().getResourceClient(LogLevel.NONE);
        LOGGER.info("creating service principal");
        final ServicePrincipal sp = azure.accessManagement().servicePrincipals().define("jp-storage-test-service-principal")
            .withNewApplication("https://testappname")
            .definePasswordCredential("password1")
                .withPasswordValue("thisisatestpassword")
                .attach().create();

        LOGGER.info("Created service principal with id: {} and application id: {}", sp.id(), sp.applicationId());
    }

Failing update with new credential call:

@Test
        //@Ignore
        public void sp_add_credential()  {
            try {
            final Azure azure = new AzureCloudProvider(cloud).getAzureClient().getResourceClient(LogLevel.NONE);
            final ServicePrincipal sp = azure.accessManagement().servicePrincipals().getByName("jp-storage-test-service-principal");

            sp.update().definePasswordCredential("temp-password").withPasswordValue(UUID.randomUUID().toString())
                .withDuration(Duration.standardHours(1)).attach().apply();
            } catch (Exception e) {
                LOGGER.warn("caught exception of type: {}, message: {}", e.getClass().getSimpleName(), e.getMessage());
            }
        }

Exception:
2020-10-10 10:43:06.347 EDT defaultContext [main                   ] [user= project=] WARN  c.c.f.v.a.a.AzureCloudProviderTests - caught exception of type: GraphErrorException, message: Status code 400, {"odata.error":{"code":"Request_BadRequest","message":{"lang":"en","value":"The property 'additionalProperties' does not exist on type 'Microsoft.DirectoryServices.PasswordCredential'. Make sure to only use property names that are defined by the type."},"requestId":"1c4cf37c-673a-4bab-96b5-77ac9869838e","date":"2020-10-10T14:43:06"}}

Removing a credential via the update path has a similar error.

Working graph api based add credential call:

@Test
        @Ignore
        public void graph_add_password() {
            try {
            String clientId = cloud.getClientId();
            List<String> scopes = new ArrayList<>();
            scopes.add("https://graph.microsoft.com/.default");

            String clientSecret = cloud.getSecretAccessKey();
            String tenant = cloud.getTenant();
            NationalCloud nationalCloud = NationalCloud.UsGovernment;

            ClientCredentialProvider authProvider = new ClientCredentialProvider(clientId, scopes, clientSecret, tenant, nationalCloud);

            IGraphServiceClient graphClient = GraphServiceClient
                            .builder()
                            .authenticationProvider(authProvider)
                            .buildClient();

            final Duration duration = Duration.standardMinutes(10);
            final DateTime now = DateTime.now();
            final Locale locale = Locale.getDefault();
            final String name = "password-timed";

            PasswordCredential passwordCredential = new PasswordCredential();
            passwordCredential.displayName = name;
            passwordCredential.startDateTime = now.toCalendar(Locale.getDefault());
            passwordCredential.endDateTime = now.plus(duration).toCalendar(locale);

            final PasswordCredential cred = graphClient.servicePrincipals("5ada2f25-2fa5-448f-9d3c-eaa8bd8e168c")
                .addPassword(passwordCredential)
                .buildRequest()
                .post();

            LOGGER.info("Created new cred with name: {} and cred: {}", name, cred.secretText);

            } finally {}
        }

It appears the graph api call goes to a POST /servicePrincipals/{id}/addPassword endpoint while the service principal update path goes to PATCH /servicePrincipals/{id} which the api documentation notes is not supported:

Using PATCH to set passwordCredential is not supported. Use the addPassword and removePassword methods to update the password for a servicePrincipal.

If there is anything else I need to provide please let me know.

xseeseesee commented 3 years ago

@paulojohnj Thanks for sharing the details! I will try to reproduce from my end and get back to you.

xseeseesee commented 3 years ago

@paulojohnj Sorry for late update. As the swagger of GraphRBAC is not updated for the new APIs of credentials related, would you like to have a try on msgraph-sdk-java. It has latest Graph APIs support by service team directly. Thanks.