spring-guides / gs-authenticating-ldap

Authenticating a User with LDAP :: Learn how to secure an application with LDAP.
https://spring.io/guides/gs/authenticating-ldap/
Apache License 2.0
98 stars 154 forks source link

To start with LDAP should not be in place #13

Closed layik closed 8 years ago

layik commented 8 years ago

The idea is of the exercise is to teach learners how LDAP can be added later, but the gradle file already includes the security dependencies in the initial folder.

pivotal-issuemaster commented 8 years ago

@layik Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

dsyer commented 8 years ago

Thanks for the suggestion. All the guides, as a matter of policy, have a complete build / dependency configuration in the initial state. It's easier to make sure they are consistent that way. It would be hard to justify this change without changing all the guides.

layik commented 8 years ago

@dsyer, I think we misunderstand each other. The tutorial is devised to show how LDAP is implemented, but the flow is wrong. When the learner is asked to first run the app, LDAP security kicks in. That is not the point of the guide. 1

layik commented 8 years ago

I will leave it there for you to decide. Thanks

pivotal-issuemaster commented 8 years ago

@layik Thank you for signing the Contributor License Agreement!

dsyer commented 8 years ago

If the text in the guide is misleading that's a different question (I haven't had time to check). Is it, do you think?

layik commented 8 years ago

@dsyer yes I just went through it myself, the flow is affected by what is in build.gradle, indeed, I just remembered, if someone is using mvn same is true for the pom.xml file.

gregturn commented 8 years ago

@layik @dsyer I can see that this guide was originally written to start from an unsecured state, then add security, a use case I've seen OFTEN in our industry.

The flow probably got interrupted when way back when, I updated all initial copies of all guides to have the same build files as the complete version.

@layik If you wish to adjust things, I think we can make an exception for this case. Tell me what you think.

layik commented 8 years ago

@gregturn I believe there should be an exception for this (including the POM file). Otherwise the guide should reflect this. Thanks.

gregturn commented 8 years ago

@layik That's what I meant to say. I agree that starting unsecured, and then securing it would be fine for this guide. Interested in tweaking things?

layik commented 8 years ago

Intention is to get back to Java/Servlets but its been years. I assume it just means removing the POM/Gradle declarations and testing both. We can ignore this pull in that case. I will try tonight.