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

security:client-registrations doesn't take propertyconfigurer properties #9491

Closed
florinco opened this issue Mar 8, 2021 · 6 comments
Closed
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: invalid An issue that we don't feel is valid

Comments

@florinco
Copy link

florinco commented Mar 8, 2021

This issue is similar to #8453
I'm using Spring Security 5.4.5.
The placeholders in security:clientRegistration tag are not resolved. Hard-coded values works fine.

I have a XML configuration containig:

<context:property-placeholder location="classpath*:oauth2.properties"/>

security:client-registrations
<security:client-registration registration-id="microsoft"
client-id="${oauth2.client.id}"
client-secret="${oauth2.client.secret}"
...
provider-id="..."/>

    <security:provider provider-id="..."
              ....
    />

</security:client-registrations>

Placeholders are resolved if used in a bean tag.

@florinco florinco added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Mar 8, 2021
@jzheaux
Copy link
Contributor

jzheaux commented Mar 8, 2021

Thanks for the report, @florinco. Since this feature has already been added, I think we'd need to see a sample to understand why it isn't working for you.

Would you be able to provide a minimal sample demonstrating the issue you are experiencing?

@jzheaux jzheaux added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 8, 2021
@florinco
Copy link
Author

florinco commented Mar 9, 2021

Thanks for reply Josh.
Basically you can use the test case from #8453 (commit 875c323f24e3a6a2ef7c2c6036bef177ad8d83df) but instead of setting the placeholders as environment variables:

  1. write them in an placeholders.properties file
  2. link the properties file to ClientRegistrationsBeanDefinitionParserTests-ClientPlaceholders.xml via <context:property-placeholder location="classpath:placeholders.properties"/>

I can confirm that the placeholders are resolved if I write them as environmental variables. They are not resolved if I store them in a .property file.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Mar 9, 2021
@jgrandja
Copy link
Contributor

@florinco The <context:property-placeholder> registers a PropertySourcesPlaceholderConfigurer @Bean, which ultimately resolves ${...} placeholders within bean definition (<b:bean ..>) property values.

The key point here is that the resolution applies only to the <b:bean ..> namespace. The <client-registrations> element is in the <security> namespace, so the PropertySourcesPlaceholderConfigurer will not apply the resolution.

You need to ensure the properties are available in the Environment, e.g. System.setProperty("oauth2.client.id", "github-client-id").

I'm going to close this issue as the behaviour is expected.

@jgrandja jgrandja added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: invalid An issue that we don't feel is valid and removed status: feedback-provided Feedback has been provided type: bug A general bug labels Mar 17, 2021
@nathan-hook
Copy link

My apologies of digging up an issue that has been closed for over a year, but I am having a terrible time getting our client -id and client-secrets values into our application securely.

From my limited understanding, it seems as though the environment variables (via $ docker exec <container-id> env) and the system property via command line (via $ ps -aux) both leak secrets.

Is there an example of how to use System.setProperty("oauth2.client.id", "github-client-id") that reads from a properties file, but is early enough in the application startup so that system properties are there prior to the <security> namespace is instantiated?

Or is there another way to inject secrets into a spring java application that won't leak secrets that I'm missing?

I am grateful for your time and all your work. It has saved us a tremendous amount of time.

@jgrandja
Copy link
Contributor

@nathan-hook

Is there an example of how to use System.setProperty("oauth2.client.id", "github-client-id")

The test ClientRegistrationsBeanDefinitionParserTests.parseWhenClientPlaceholdersThenResolvePlaceholders() demonstrates this. This enhancement was added in gh-8453 via commit 17f1540

@ychernysh
Copy link

Hi @jgrandja , does this solution work only for properties supplied via JVM properties? I want my properties to be supplied from a factory-method created bean like this:

<bean id="appProperties" class="org.example.config.PropertiesFactory" factory-method="getProperties"/>
<context:property-placeholder properties-ref="appProperties"/>
package org.example.config;

import java.util.Properties;

public class PropertiesFactory {
    public static Properties getProperties() {
        Properties properties = new Properties();
        properties.setProperty("person.name", "Alex");
        properties.setProperty("client.id", "my-client-id");
        return properties;
    }
}

But this doesn't work. The property is not resolved here:

<client-registration client-id="${client.id}" ... />

But it is still resolved in common beans:

    <bean id="person" class="org.example.model.Person">
        <constructor-arg name="name" value="${person.name}"/>
    </bean>

I'm not setting the property via System.setProperty, I want them to be created in the factory method above. Is that possible? Am I doing anything wrong? I'm on the Spring Security 6.4.1.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

6 participants