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

Spring's impersonation does not work on Vaadin #20495

Open
tbee opened this issue Nov 18, 2024 · 11 comments · May be fixed by vaadin/docs#4098
Open

Spring's impersonation does not work on Vaadin #20495

tbee opened this issue Nov 18, 2024 · 11 comments · May be fixed by vaadin/docs#4098

Comments

@tbee
Copy link

tbee commented Nov 18, 2024

Description of the bug

Spring offers a default way of an administrator impersonating a regular user. This seems not to work on Vaadin-on-Spring because Authorization seems not to be setup yet in the start-impersonating request. More here https://vaadin.com/forum/t/how-to-do-impersonation-using-spring-security/167804

Expected behavior

Well, it should work 😄

Minimal reproducible example

On a Vaadin-on-Spring application with Spring security and login enabled: configure the SwitchUserServlet as per one of the many examples, preferable on GET (which makes test easier) and attempt an impersonation. Probably VaadinAwareSecurityContextHolderStrategy should be set as the strategy on the filter (but won't fix the problem).

https://stackoverflow.com/questions/72378146/user-impersonation-with-spring-security

Versions

  • Vaadin / Flow version: 24.5.4
  • Java version: 21
  • OS version: not relevant
  • Browser version (if applicable): not relevant
  • Application Server (if applicable): not relevant
  • IDE (if applicable): not relevant
@mshabarov
Copy link
Contributor

This sounds to me more like an enhancement, however, we have to double check our integration with Spring Security, maybe there is something blocking this feature to work.

@tbee
Copy link
Author

tbee commented Nov 19, 2024

Assuming the integration with Spring security is supposed to encompass all standard functionality, than this should be supported IMHO.

@mshabarov mshabarov moved this from 🅿️Parking lot - under consideration to 🪵Product backlog in Vaadin Flow ongoing work (Vaadin 10+) Nov 22, 2024
@svasic
Copy link

svasic commented Jan 8, 2025

@tbee JFYI, impersonation worked for me on V14 and V23, but I can't make it work on V24 too.

@mshabarov mshabarov moved this from 🪵Product backlog to 🟢Ready to Go in Vaadin Flow ongoing work (Vaadin 10+) Jan 22, 2025
@mshabarov mshabarov moved this from 🟢Ready to Go to ⚒️ In progress in Vaadin Flow ongoing work (Vaadin 10+) Jan 22, 2025
@mshabarov mshabarov self-assigned this Jan 22, 2025
@mshabarov
Copy link
Contributor

mshabarov commented Jan 24, 2025

I did a quick example project based on Vaadin 23 that uses Spring Security and SwitchUserGrantedAuthority, but doesn't use SwitchUserFilter which makes it possible to impersonate user by just clicking on a button.

First commit in this branch uses SwitchUserFilter and it also works good.

Next step is to rebase this to Vaadin 24 and figure out why we got an exception - random clues are major version jump of the Spring Security (Spring 5 -> Spring 6) or some new features in Vaadin 24 that added more logic to Spring Security integration.

@tbee
Copy link
Author

tbee commented Jan 26, 2025

Hm, personally I'd probably prefer the solution with just a button. But it seems to be a lot of spring code of which I'm not certain how standard it is. Just redirecting to an URL is the approach with the lower coupling.

@mshabarov mshabarov removed their assignment Jan 28, 2025
@mshabarov mshabarov moved this from ⚒️ In progress to 🟢Ready to Go in Vaadin Flow ongoing work (Vaadin 10+) Jan 28, 2025
@svasic
Copy link

svasic commented Jan 29, 2025

It looks like SwitchUserFilter does not use same security context configuration as defined in VaadinWebSecurity (specifically requireExplicitSave(false)) . For now, I managed to achieve impersonation by defining CustomSwitchUserFilter that extends SwitchUserFilter and overriding needed methods. In method attemptSwitchUser after publishing event i added this code SecurityContextHolder.getContext().setAuthentication(targetUserRequest); and it is working as before. It is probably misconfiguration of spring security, but I have not found any other way of achieving this for now.

@caalador caalador self-assigned this Jan 29, 2025
@caalador caalador moved this from 🟢Ready to Go to ⚒️ In progress in Vaadin Flow ongoing work (Vaadin 10+) Jan 29, 2025
@caalador
Copy link
Contributor

caalador commented Jan 30, 2025

What I found out was that with v24.6 the userFilter uses the wrong securityContextHolderStrategy.
Making the following setup in crm tutorial has impersonation working ok.

In short the changes are:
add into VaadinWebSecurity config

  @Autowired
  private ApplicationContext applicationContext;

  @Bean
  public SwitchUserFilter switchUserFilter() {
    SwitchUserFilter filter = new SwitchUserFilter();
    filter.setSecurityContextHolderStrategy(
            applicationContext.getBean(VaadinAwareSecurityContextHolderStrategy.class));
    filter.setUserDetailsService(userDetailsService());
    filter.setSwitchUserMatcher(antMatcher(HttpMethod.GET, "/impersonate"));
    filter.setSwitchFailureUrl("/switchUser");
    filter.setExitUserMatcher(antMatcher(HttpMethod.GET, "/impersonate/exit"));
    filter.setTargetUrl("/");
    return filter;
  }

  @Override
  protected void configure(HttpSecurity http) throws Exception {
    http.authorizeHttpRequests(auth -> auth.requestMatchers(new AntPathRequestMatcher("/impersonate")).hasAnyRole("ADMIN", "PREVIOUS_ADMINISTRATOR"));
    http.authorizeHttpRequests(auth -> auth.requestMatchers(new AntPathRequestMatcher("/impersonate/exit")).hasRole("PREVIOUS_ADMINISTRATOR"));
    ...
  }

Impersonation of use

        Button impersonate = new Button("Impersonate user", e -> {
            getUI().ifPresent(ui -> ui.getPage().setLocation("/impersonate?username=user"));
        });

Exit impersonation

        Button exitImpersonation = new Button("Exit impersonation", e ->
                getUI().ifPresent(ui -> ui.getPage().setLocation("/impersonate/exit")));

@mshabarov
Copy link
Contributor

A possible solution could be modifying the SwitchUserFilter if it's present in the project's config:

@Bean
@ConditionalOnBean(SwitchUserFilter.class)
public SwitchUserFilter customizedSwitchUserFilter(SwitchUserFilter existingFilter) {
    // set Vaadin security context
    return existingFilter;
}

But would be good to figure out why the Vaadin context is not injected without this hack.

@caalador
Copy link
Contributor

The main issue is that when the Filter initializes the securitycontextholderstrategy gets chosen by default as ThreadLocalSecurityContextHolderStrategy which doesn't contain any authentication as vaadin stores that in the VaadinAwareSecurityContextHolderStrategy.

Also it seems it's a timing issue from #14631 where the security context was moved from a set during configuration to being a Bean.

Seems the correct fix for this would be to set up the userFilter as:

  @Bean
  @DependsOn("VaadinSecurityContextHolderStrategy")
  public SwitchUserFilter switchUserFilter() {
    SwitchUserFilter filter = new SwitchUserFilter();
    filter.setUserDetailsService(userDetailsService());
    filter.setSwitchUserMatcher(antMatcher(HttpMethod.GET, "/impersonate"));
    filter.setSwitchFailureUrl("/switchUser");
    filter.setExitUserMatcher(antMatcher(HttpMethod.GET, "/impersonate/exit"));
    filter.setTargetUrl("/");
    return filter;
  }

This way the context is correct when the switch user filter is generated.

@mshabarov
Copy link
Contributor

How about combining these two annotations together to embed this fix to the framework? (not tested though)

@Bean
@ConditionalOnBean(SwitchUserFilter.class)
@DependsOn("VaadinSecurityContextHolderStrategy")  // Ensure it initializes after VaadinSecurityContextHolderStrategy
public SwitchUserFilter projectSwitchUserFilter(SwitchUserFilter switchUserFilter) {
    // no modifications, just return
    return switchUserFilter;
}

@caalador
Copy link
Contributor

I would perhaps just go with documentation and an it test module in Flow so we catch any change to functionality on updates.

caalador added a commit to vaadin/docs that referenced this issue Jan 31, 2025
Add to the security document
documentation on the impersonation
feature in Spring.

Closes vaadin/flow#20495
@caalador caalador linked a pull request Jan 31, 2025 that will close this issue
@mshabarov mshabarov moved this from ⚒️ In progress to 🔎Iteration reviews in Vaadin Flow ongoing work (Vaadin 10+) Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🔎Iteration reviews
Development

Successfully merging a pull request may close this issue.

4 participants