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

SEC-703: Expose customization of SQL used by <jdbc-user-service> #965

Closed
spring-issuemaster opened this Issue Mar 8, 2008 · 6 comments

Comments

Projects
None yet
1 participant
@spring-issuemaster

spring-issuemaster commented Mar 8, 2008

Craig Walls(Migrated from SEC-703) said:

JdbcUserDetailsManager (and its parent JdbcDaoImpl), the bean underneath offers setter methods for providing custom SQL for the various operations that it performs. None of these setters, however, are exposed through . Therefore is useless to anyone whose user details are kept in tables other than the defaults assumed by JdbcUserDetailsManager.

This issue suggests that those properties should be exposed through . Perhaps something like…

@spring-issuemaster

This comment has been minimized.

spring-issuemaster commented Mar 8, 2008

Craig Walls said:

Attached patch for supporting customization of SQL through for user authentication, user authorization, and user group authorization (the three customizations offered through org.springframework.security.userdetails.jdbc.JdbcDaoImpl).

What this patch DOESN’T do is to support customization of the SQL used in org.springframework.security.userdetails.jdbc.JdbcUserDetailsManager (insert, delete, and update SQL).

@spring-issuemaster

This comment has been minimized.

spring-issuemaster commented Apr 4, 2008

Luke Taylor said:

I think this is probably worth doing because jdbc-user-service won’t be of much practical use unless people can customize the SQL. However, the amount of customization that will be possible is limited – loading extra columns won’t be possible. There is of course nothing to prevent a user from actually using a JbdcDaoImpl bean directly or (more flexibly) a subclass of that which returns their own UserDetails object and putting the Id of that bean in their user-service-ref attribute. JbdcDaoImpl configuration is not very verbose so it’s not a big deal to mix traditional bean config with namespace config here.

@spring-issuemaster

This comment has been minimized.

spring-issuemaster commented Apr 13, 2008

Luke Taylor said:

I’ve added the three suggested attributes to match the query properties in JdbcDaoImpl.

@spring-issuemaster

This comment has been minimized.

spring-issuemaster commented Apr 15, 2008

Triqui Galletas said:

Sorry, maybe I’m missing something here, but why not add userTable, userNameCol, userCredCol, userRoleTable, roleNameCol, and so on like the Realm component in the tomcat security configuration?
(http://tomcat.apache.org/tomcat-5.5-doc/config/realm.html)

@spring-issuemaster

This comment has been minimized.

spring-issuemaster commented Apr 15, 2008

Craig Walls said:

Because my database schema may not be so simple as to include the username, password, and enabled status in a single user table.

For that matter, I may not even care to take advantage of the enabled field, but Spring Security still requires it. Being able to specify SQL gives me the ultimate flexibility so that I can write queries like “select student_id, password, true from students where student_id=?”. In this case, I’ve effectively set the enabled flag to always be true for all users.

Another reason to be able to specify SQL is that it is consistent with how JdbcDaoImpl already works. Since is just a configuration abstraction over JdbcDaoImpl, I argue that the configuration options should be consistent between the two.

@spring-issuemaster

This comment has been minimized.

spring-issuemaster commented Apr 17, 2008

Triqui Galletas said:

Well, in fact what I would really like is the property rolePrefix to be exposed, cause even with this fix applied I still have to define my authentication provider like this:

```

```

I don’t think it will happen, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment