Skip to content

failed to make “max-sessions” field configurable #9202

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

Closed
mazenaissa opened this issue Nov 13, 2020 · 10 comments
Closed

failed to make “max-sessions” field configurable #9202

mazenaissa opened this issue Nov 13, 2020 · 10 comments
Assignees
Labels
in: config An issue in spring-security-config type: enhancement A general enhancement

Comments

@mazenaissa
Copy link
Contributor

mazenaissa commented Nov 13, 2020

Hello,
I am using spring-security 5.0.6 and I want to make max-sessions field configurable from a properties file
So I did this :

<security:session-management>
    <security:concurrency-control max-sessions="${security.maxSessions}" session-registry-ref="sessionRegistry""/>
</security:session-management>

But i get this error :
Multiple annotations found at this line:
- cvc-datatype-valid.1.2.1: '${security.maxSessions}' is not a valid value for 'integer'.
- cvc-attribute.3: The value '${security.maxSessions}' of attribute 'max-sessions' on element 'security:concurrency-control' is not valid with respect to its type, 'integer'.

Is there any solution to make it configurable ?
Thanks,

@mazenaissa mazenaissa added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Nov 13, 2020
@mazenaissa
Copy link
Contributor Author

mazenaissa commented Nov 13, 2020

I suggest to change the type of max-sessions from "xs:integer" to "xs:token" or "xs:string" in http://www.springframework.org/schema/security/spring-security.xsd , so we can configure it dynamically from properties.

@jzheaux
Copy link
Contributor

jzheaux commented Nov 24, 2020

@mazenaissa, thanks for the suggestion. Would you be able to submit a PR to add the feature?

You can see an example of resolving placeholders in HeadersBeanDefinitionParser.

@jzheaux jzheaux self-assigned this Nov 24, 2020
@jzheaux jzheaux added type: enhancement A general enhancement in: config An issue in spring-security-config and removed status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Nov 24, 2020
@mazenaissa
Copy link
Contributor Author

mazenaissa commented Nov 25, 2020

Hello @jzheaux
When trying to submit the PR I faced this error : git-receive-pack not permitted
The feature can be added on which versions ? From 5.0 to 5.5 ?
And in order to change the type of max-sessions to token I must change the spring-security.rnc and spring-security.xsd right ?
Thanks,

@jzheaux
Copy link
Contributor

jzheaux commented Nov 25, 2020

Good questions, @mazenaissa, let's see if I can help.

When trying to submit the PR I faced this error : git-receive-pack not permitted

This usually means you don't have permissions. Are you pushing to your fork of the repo?

The feature can be added on which versions ? From 5.0 to 5.5 ?

Since it is a new feature, let's add it to 5.5

And in order to change the type of max-sessions to token I must change the spring-security.rnc and spring-security.xsd right ?

You should be able to create the rnc and then run the rncToXsd gradle task:

./gradlew :spring-security-config:rncToXsd

@mazenaissa
Copy link
Contributor Author

Thanks for your reply.
1- I am pushing to spring security project, branch master.
2- If it will be added only in v5.5, do the default spring-security.xsd will have it ? As I am using spring security v5.0.6 jars and importing the xsd in my XML configuration without version, e,g spring-security-5.5.xsd.
3- So I need only to commit the change on the rnc file and test the generation of xsd to see if everything works well ?

@jzheaux
Copy link
Contributor

jzheaux commented Nov 25, 2020

1- I am pushing to spring security project, branch master.

To make a contribution, please follow these steps:

  1. Fork the project and clone it locally
  2. Create a branch, you might name it gh-9202:
git checkout -b gh-9202
  1. Make your changes and commit
  2. Push to your fork
  3. Open a PR to merge your fork's branch with Spring Security master

2- If it will be added only in v5.5, do the default spring-security.xsd will have it ?

spring-security.xsd points to the latest version of the schema.

But, 5.0.6 won't resolve placeholders for max-sessions, even when pointing to the latest schema. The reason is that it's the Java code that resolves the placeholder, not the schema. The first version that can use your feature is 5.5.

3- So I need only to commit the change on the rnc file and test the generation of xsd to see if everything works well ?

To add this support, you should do the following:

  1. Update spring-security-5.5.rnc
  2. Run ./gradlew :spring-security-config:rncToXsd to update the XSD
  3. Update HttpConfigurationBuilder.java to resolve the placeholder, similar to how it's done in HeaderBeanDefinitionParser.

mazenaissa added a commit to mazenaissa/spring-security that referenced this issue Dec 9, 2020
set max-session to token type and resolve placeholder

Closes spring-projectsgh-9202
@mazenaissa
Copy link
Contributor Author

Hello @jzheaux
Could you please see why the PR's pipeline fails ?
Thanks,

@mazenaissa
Copy link
Contributor Author

Hello @jzheaux
The problem of unauthorized is resolved, I retried pushing the branch and the pipeline succeeded.
You can accept the PR :)
Thanks,

mazenaissa added a commit to mazenaissa/spring-security that referenced this issue Jan 5, 2021
@mazenaissa
Copy link
Contributor Author

Hello @jzheaux
All changes are in this PR #9328
Thanks,

@jzheaux jzheaux closed this as completed in c907838 Jan 5, 2021
jzheaux added a commit that referenced this issue Jan 5, 2021
@mazenaissa
Copy link
Contributor Author

Hello @jzheaux
Thanks for the copyright year update, you know it's the start of the new year and that kind of errors happens a lot hh.
Thanks,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: config An issue in spring-security-config type: enhancement A general enhancement
Projects
None yet
2 participants