AppFuse

UserSecurityAdvice not being applied to UserManager

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.9.4, 2.0-M2
  • Fix Version/s: 1.9.4, 2.0-M2
  • Component/s: Security
  • Labels:
    None

Description

UserSecurityAdvice is not being applied to UserManager because of a conflict of the advisorId in applicationContext-service.xml and with the bean id in security.xml. Both have an entry for "userManagerSecurity" The bean from security.xml seems to dominate and thus the UserSecurityAdvice is not applied. The current unit tests do not fail on this problem because the unit test is not configured with the same naming conflicts. Changing the name of the id in one or the other fixes this problem.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Matt Raible added a comment -

Do you have a test case or scenario to prove that this happens?

Show
Matt Raible added a comment - Do you have a test case or scenario to prove that this happens?
Hide
Create a new review for changeset 6f6d6d83ed52dc3858e35002a1874fdf36ad1684
Matt Raible <matt@raibledesigns.com> submitted changeset 6f6d6d83ed52dc3858e35002a1874fdf36ad1684 to wicket in appfuse (1 file) - 22/Oct/06 11:08 AM

APF-472: Fixed issue with UserSecurityAdvice not being applied to UserManager

Show
Create a new review for changeset 6f6d6d83ed52dc3858e35002a1874fdf36ad1684
Matt Raible <matt@raibledesigns.com> submitted changeset 6f6d6d83ed52dc3858e35002a1874fdf36ad1684 to wicket in appfuse (1 file) - 22/Oct/06 11:08 AM
Matt Raible made changes -
Field Original Value New Value
Fix Version/s 1.9.4 [ 10090 ]
Fix Version/s 2.0 [ 10005 ]
Affects Version/s 1.9.4 [ 10090 ]
Matt Raible made changes -
Component/s Security [ 10006 ]
Matt Raible made changes -
Status Open [ 1 ] Resolved [ 5 ]
Resolution Fixed [ 1 ]
Hide
Permalink
Darren Salomons added a comment -

Well spring would have thrown an error if the two were in the same xml file since it wouldn't validate having non-unique keys. But since they are in separate xml files spring is very quiet about the conflict. In fact you would have to be logging up to the INFO threshold to see this... it wasnt until I turned on INFO that I saw exactly what the problem was. I had done a small adhoc attack on upgrading the users role using the firefox Web Developer Toolbar. This toolbar allows you to edit the html live through a sidebar which I can then modify the contents of the hidden input variables(changing user to admin) and then just hitting submit. A very seemless way of testing form hacking. (if you have never used the toolbar its under Miscellaneous->Edit Html) I noticed that it allowed me to upgrade. After messing with the execution pointcuts for a while I found the real problem and this was the output:

INFO - DefaultListableBeanFactory.registerBeanDefinition(327) | Overriding bean definition for bean 'userManagerSecurity': replacing [Root bean: class [org.springframework.aop.aspectj.AspectJPointcutAdvisor]; scope=singleton; abstract=false; lazyInit=false; autowireCandidate=true; autowireMode=0; dependencyCheck=0; factoryBeanName=null; factoryMethodName=null; initMethodName=null; destroyMethodName=null] with [Root bean: class [org.acegisecurity.intercept.method.aopalliance.MethodSecurityInterceptor]; scope=singleton; abstract=false; lazyInit=false; autowireCandidate=true; autowireMode=0; dependencyCheck=0; factoryBeanName=null; factoryMethodName=null; initMethodName=null; destroyMethodName=null; defined in URL file:/Users/digid/Documents/workspace/.metadata/.plugins/org.eclipse.wst.server.core/tmp0/webapps/appfuse-svn/WEB-INF/classes/security.xml]

note how the first bean is saying it will be replced by the second by the one from security.xml

Show
Darren Salomons added a comment - Well spring would have thrown an error if the two were in the same xml file since it wouldn't validate having non-unique keys. But since they are in separate xml files spring is very quiet about the conflict. In fact you would have to be logging up to the INFO threshold to see this... it wasnt until I turned on INFO that I saw exactly what the problem was. I had done a small adhoc attack on upgrading the users role using the firefox Web Developer Toolbar. This toolbar allows you to edit the html live through a sidebar which I can then modify the contents of the hidden input variables(changing user to admin) and then just hitting submit. A very seemless way of testing form hacking. (if you have never used the toolbar its under Miscellaneous->Edit Html) I noticed that it allowed me to upgrade. After messing with the execution pointcuts for a while I found the real problem and this was the output: INFO - DefaultListableBeanFactory.registerBeanDefinition(327) | Overriding bean definition for bean 'userManagerSecurity': replacing [Root bean: class [org.springframework.aop.aspectj.AspectJPointcutAdvisor] ; scope=singleton; abstract=false; lazyInit=false; autowireCandidate=true; autowireMode=0; dependencyCheck=0; factoryBeanName=null; factoryMethodName=null; initMethodName=null; destroyMethodName=null] with [Root bean: class [org.acegisecurity.intercept.method.aopalliance.MethodSecurityInterceptor] ; scope=singleton; abstract=false; lazyInit=false; autowireCandidate=true; autowireMode=0; dependencyCheck=0; factoryBeanName=null; factoryMethodName=null; initMethodName=null; destroyMethodName=null; defined in URL file:/Users/digid/Documents/workspace/.metadata/.plugins/org.eclipse.wst.server.core/tmp0/webapps/appfuse-svn/WEB-INF/classes/security.xml ] note how the first bean is saying it will be replced by the second by the one from security.xml
Matt Raible made changes -
Comment [ https://www.cbil.upenn.edu/tracker/attachment.cgi?id=46.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=47.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=48.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=49.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=50.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=51.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=52.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=53.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=54.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=55.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=56.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=57.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=58.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=59.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=60.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=61.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=62.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=63.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=64.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=65.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=66.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=67.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=68.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=69.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=70.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=71.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=72.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=73.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=74.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=75.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=76.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=77.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=78.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=79.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=80.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=81.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=82.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=83.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=84.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=85.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=86.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=87.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=88.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=89.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=90.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=91.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=92.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=93.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=94.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=95.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=96.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=97.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=98.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=99.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=100.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=101.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=102.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=103.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=104.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=105.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=106.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=107.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=108.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=109.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=110.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=111.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=112.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=113.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=114.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=115.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=116.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=117.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=118.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=119.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=120.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=121.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=122.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=123.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=124.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=125.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=126.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=127.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=128.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=129.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=130.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=131.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=132.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=133.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=134.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=135.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=136.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=137.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=138.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=139.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=140.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=141.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=142.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=143.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=144.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=145.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=146.html - https://www.cbil.upenn.edu/tracker/attachment.cgi?id=147.html ]

People

  • Assignee:
    Matt Raible
    Reporter:
    Darren Salomons
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: