Skip to content

Commit

Permalink
PLUGINAPI-42 Deprecate javax.servlet usage in org.sonar.api.security …
Browse files Browse the repository at this point in the history
…package
  • Loading branch information
antoine-vinot-sonarsource authored and jacek-poreda-sonarsource committed Apr 18, 2023
1 parent f5e6e85 commit 8c10020
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 11 deletions.
31 changes: 31 additions & 0 deletions plugin-api/src/main/java/org/sonar/api/security/Authenticator.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import javax.servlet.http.HttpServletRequest;
import org.sonar.api.ExtensionPoint;
import org.sonar.api.server.ServerSide;
import org.sonar.api.server.http.HttpRequest;
import org.sonar.api.server.http.JavaxHttpRequest;

import static java.util.Objects.requireNonNull;

Expand All @@ -43,13 +45,31 @@ public abstract class Authenticator {
public static final class Context {
private String username;
private String password;
/**
* @deprecated since 9.16 use {@link #httpRequest} instead
*/
@Deprecated(since = "9.16", forRemoval = true)
private HttpServletRequest request;
private HttpRequest httpRequest;

/**
* @deprecated since 9.16 use {@link #Context(String, String, HttpRequest)} instead
*/
@Deprecated(since = "9.16", forRemoval = true)
public Context(@Nullable String username, @Nullable String password, HttpServletRequest request) {
requireNonNull(request);
this.request = request;
this.username = username;
this.password = password;
this.httpRequest = new JavaxHttpRequest(request);
}

public Context(@Nullable String username, @Nullable String password, HttpRequest httpRequest) {
requireNonNull(httpRequest);
this.httpRequest = httpRequest;
this.username = username;
this.password = password;
this.request = (HttpServletRequest) httpRequest.getRawRequest();
}

/**
Expand All @@ -66,8 +86,19 @@ public String getPassword() {
return password;
}

/**
* @deprecated since 9.16. Use {@link #getHttpRequest()} instead.
*/
@Deprecated(since = "9.16", forRemoval = true)
public HttpServletRequest getRequest() {
return request;
}

/**
* @since 9.16
*/
public HttpRequest getHttpRequest() {
return httpRequest;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import java.util.Collection;
import javax.annotation.CheckForNull;
import javax.servlet.http.HttpServletRequest;
import org.sonar.api.server.http.HttpRequest;
import org.sonar.api.server.http.JavaxHttpRequest;

/**
* Note that prefix "do" for names of methods is reserved for future enhancements, thus should not be used in subclasses.
Expand All @@ -45,19 +47,46 @@ public Collection<String> doGetGroups(Context context) {

public static final class Context {
private String username;
/**
* @deprecated since 9.16 use {@link #httpRequest} instead
*/
@Deprecated(since = "9.16", forRemoval = true)
private HttpServletRequest request;
private HttpRequest httpRequest;

/**
* @deprecated since 9.16 use {@link #Context(String, HttpRequest)} instead
*/
@Deprecated(since = "9.16", forRemoval = true)
public Context(String username, HttpServletRequest request) {
this.username = username;
this.request = request;
this.httpRequest = new JavaxHttpRequest(request);
}

public Context(String username, HttpRequest httpRequest) {
this.username = username;
this.httpRequest = httpRequest;
this.request = (HttpServletRequest) httpRequest.getRawRequest();
}

public String getUsername() {
return username;
}

/**
* @deprecated since 9.16. Use {@link #getHttpRequest()} instead.
*/
@Deprecated(since = "9.16", forRemoval = true)
public HttpServletRequest getRequest() {
return request;
}

/**
* @since 9.16
*/
public HttpRequest getHttpRequest() {
return httpRequest;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@

import javax.annotation.Nullable;
import javax.servlet.http.HttpServletRequest;
import org.sonar.api.server.http.HttpRequest;
import org.sonar.api.server.http.JavaxHttpRequest;

/**
* Note that prefix "do" for names of methods is reserved for future enhancements, thus should not be used in subclasses.
Expand All @@ -42,19 +44,46 @@ public UserDetails doGetUserDetails(Context context) {

public static final class Context {
private String username;
/**
* @deprecated since 9.16 use {@link #httpRequest} instead
*/
@Deprecated(since = "9.16", forRemoval = true)
private HttpServletRequest request;
private HttpRequest httpRequest;

/**
* @deprecated since 9.16 use {@link #Context(String, HttpRequest)} instead
*/
@Deprecated(since = "9.16", forRemoval = true)
public Context(@Nullable String username, HttpServletRequest request) {
this.username = username;
this.request = request;
this.httpRequest = new JavaxHttpRequest(request);
}

public Context(@Nullable String username, HttpRequest httpRequest) {
this.username = username;
this.httpRequest = httpRequest;
this.request = (HttpServletRequest) httpRequest.getRawRequest();
}

public String getUsername() {
return username;
}

/**
* @deprecated since 9.16. Use {@link #getHttpRequest()} instead.
*/
@Deprecated(since = "9.16", forRemoval = true)
public HttpServletRequest getRequest() {
return request;
}

/**
* @since 9.16
*/
public HttpRequest getHttpRequest() {
return httpRequest;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,13 @@
package org.sonar.api.security;

import com.google.common.base.Preconditions;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import javax.annotation.Nullable;
import javax.servlet.http.HttpServletRequest;
import org.junit.Test;
import org.sonar.api.server.http.HttpRequest;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
Expand All @@ -40,7 +39,7 @@ public void doGetGroupsNoOverride() {

String userName = "foo";
assertThat(groupsProvider.doGetGroups(new ExternalGroupsProvider.Context(userName,
mock(HttpServletRequest.class)))).isNull();
mock(HttpRequest.class)))).isNull();
}

@Test
Expand All @@ -51,7 +50,7 @@ public void doGetGroupsTests() {
@Override
public Collection<String> doGetGroups(Context context) {
Preconditions.checkNotNull(context.getUsername());
Preconditions.checkNotNull(context.getRequest());
Preconditions.checkNotNull(context.getHttpRequest());

return userGroupsMap.get(context.getUsername());
}
Expand All @@ -63,7 +62,7 @@ public Collection<String> doGetGroups(Context context) {
private static void runDoGetGroupsTests(ExternalGroupsProvider groupsProvider, Map<String, Collection<String>> userGroupsMap) {
for (Map.Entry<String, Collection<String>> userGroupMapEntry : userGroupsMap.entrySet()) {
Collection<String> groups = groupsProvider.doGetGroups(new ExternalGroupsProvider.Context(
userGroupMapEntry.getKey(), mock(HttpServletRequest.class)));
userGroupMapEntry.getKey(), mock(HttpRequest.class)));
assertThat(groups).isEqualTo(userGroupMapEntry.getValue());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@

import com.google.common.base.Preconditions;
import org.junit.Test;

import javax.servlet.http.HttpServletRequest;
import org.sonar.api.server.http.HttpRequest;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
Expand All @@ -35,14 +34,14 @@ public void doGetUserDetails() {
@Override
public UserDetails doGetUserDetails(Context context) {
Preconditions.checkNotNull(context.getUsername());
Preconditions.checkNotNull(context.getRequest());
Preconditions.checkNotNull(context.getHttpRequest());
UserDetails user = new UserDetails();
user.setName(context.getUsername());
user.setEmail("[email protected]");
return user;
}
};
UserDetails user = provider.doGetUserDetails(new ExternalUsersProvider.Context("foo", mock(HttpServletRequest.class)));
UserDetails user = provider.doGetUserDetails(new ExternalUsersProvider.Context("foo", mock(HttpRequest.class)));

assertThat(user.getName()).isEqualTo("foo");
assertThat(user.getEmail()).isEqualTo("[email protected]");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

import org.junit.Test;

import javax.servlet.http.HttpServletRequest;
import org.sonar.api.server.http.HttpRequest;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
Expand Down Expand Up @@ -52,7 +52,7 @@ public LoginPasswordAuthenticator getLoginPasswordAuthenticator() {
}
};
Authenticator proxy = realm.doGetAuthenticator();
Authenticator.Context context = new Authenticator.Context("foo", "bar", mock(HttpServletRequest.class));
Authenticator.Context context = new Authenticator.Context("foo", "bar", mock(HttpRequest.class));
proxy.doAuthenticate(context);

verify(deprecatedAuthenticator).authenticate("foo", "bar");
Expand Down

0 comments on commit 8c10020

Please sign in to comment.