Skip to content

Commit

Permalink
Closes #793 Allow API access to Elide for services without user authe…
Browse files Browse the repository at this point in the history
…ntication
  • Loading branch information
bukajsytlos committed Nov 24, 2023
1 parent 555c745 commit 4828ec1
Show file tree
Hide file tree
Showing 22 changed files with 223 additions and 129 deletions.
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package com.faforever.api.user;

import com.faforever.api.AbstractIntegrationTest;
import com.faforever.api.security.FafRole;
import org.junit.jupiter.api.Test;

import java.util.Set;

import static com.faforever.api.data.domain.GroupPermission.ROLE_READ_ACCOUNT_PRIVATE_DETAILS;
import static com.faforever.api.data.domain.GroupPermission.ROLE_USER;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.is;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
Expand All @@ -29,6 +31,9 @@ public void withActiveUserGetResult() throws Exception {
.andExpect(jsonPath("$.data.attributes.userName", is(AUTH_ACTIVE_USER)))
.andExpect(jsonPath("$.data.attributes.email", is("[email protected]")))
.andExpect(jsonPath("$.data.attributes.permissions",
containsInAnyOrder(ROLE_READ_ACCOUNT_PRIVATE_DETAILS, "ROLE_" + ROLE_READ_ACCOUNT_PRIVATE_DETAILS)));
containsInAnyOrder(
ROLE_READ_ACCOUNT_PRIVATE_DETAILS, FafRole.ROLE_PREFIX + ROLE_READ_ACCOUNT_PRIVATE_DETAILS,
ROLE_USER, FafRole.ROLE_PREFIX + ROLE_USER
)));
}
}
6 changes: 3 additions & 3 deletions src/inttest/java/com/faforever/api/utils/OAuthHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

import com.faforever.api.data.domain.Player;
import com.faforever.api.player.PlayerRepository;
import com.faforever.api.security.FafAuthenticationToken;
import com.faforever.api.security.FafRole;
import com.faforever.api.security.FafScope;
import com.faforever.api.security.FafUserAuthenticationToken;
import org.jetbrains.annotations.NotNull;
import org.springframework.stereotype.Component;
import org.springframework.test.web.servlet.request.RequestPostProcessor;
Expand Down Expand Up @@ -32,7 +32,7 @@ public RequestPostProcessor addBearerTokenForUser(int userId, @NotNull Set<Strin

var fafScopes = scopes.stream().map(FafScope::new).toList();

return authentication(new FafAuthenticationToken(userId, user.getLogin(), fafScopes, roles));
return authentication(new FafUserAuthenticationToken(userId, user.getLogin(), fafScopes, roles));
}

public RequestPostProcessor addBearerToken(
Expand All @@ -43,6 +43,6 @@ public RequestPostProcessor addBearerToken(
var fafScopes = scopes.stream().map(FafScope::new).toList();
var fafRoles = roles.stream().map(FafRole::new).toList();

return authentication(new FafAuthenticationToken(userId, "[undefined]", fafScopes, fafRoles));
return authentication(new FafUserAuthenticationToken(userId, "[undefined]", fafScopes, fafRoles));
}
}
55 changes: 29 additions & 26 deletions src/main/java/com/faforever/api/coturn/CoturnServerEnricher.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@

import com.faforever.api.config.FafApiProperties;
import com.faforever.api.data.domain.CoturnServer;
import com.faforever.api.security.FafAuthenticationToken;
import com.faforever.api.security.FafUserAuthenticationToken;
import com.faforever.api.security.UserSupplier;
import jakarta.inject.Inject;
import jakarta.persistence.PostLoad;
import org.apache.commons.codec.digest.HmacAlgorithms;
import org.apache.commons.codec.digest.HmacUtils;
import org.springframework.stereotype.Component;

import jakarta.inject.Inject;
import jakarta.persistence.PostLoad;
import java.net.URI;
import java.util.Base64;
import java.util.HashSet;
Expand All @@ -29,28 +29,31 @@ public void init(FafApiProperties fafApiProperties, UserSupplier userSupplier) {

@PostLoad
public void enhance(CoturnServer coturnServer) {
FafAuthenticationToken fafAuthenticationToken = userSupplier.get();

// Build hmac verification as described here:
// https://github.com/coturn/coturn/blob/f67326fe3585eafd664720b43c77e142d9bed73c/README.turnserver#L710
long timestamp = System.currentTimeMillis() / 1000 + fafApiProperties.getCoturn().getTokenLifetimeSeconds();
String tokenName = String.format("%d:%d", timestamp, fafAuthenticationToken.getUserId());

String token = Base64.getEncoder().encodeToString(new HmacUtils(HmacAlgorithms.HMAC_SHA_1, coturnServer.getKey()).hmac(tokenName));

String host = coturnServer.getHost();
if (coturnServer.getPort() != null) {
host += ":" + coturnServer.getPort();
}

Set<URI> urls = new HashSet<>();
urls.add(URI.create("turn://%s?transport=tcp".formatted(host)));
urls.add(URI.create("turn://%s?transport=udp".formatted(host)));
urls.add(URI.create("turn://%s".formatted(host)));

coturnServer.setUrls(urls);
coturnServer.setCredentialType("token");
coturnServer.setCredential(token);
coturnServer.setUsername(tokenName);
userSupplier.get()
.filter(o -> FafUserAuthenticationToken.class.isAssignableFrom(o.getClass()))
.map(FafUserAuthenticationToken.class::cast)
.ifPresent(fafUserAuthenticationToken -> {
// Build hmac verification as described here:
// https://github.com/coturn/coturn/blob/f67326fe3585eafd664720b43c77e142d9bed73c/README.turnserver#L710
long timestamp = System.currentTimeMillis() / 1000 + fafApiProperties.getCoturn().getTokenLifetimeSeconds();
String tokenName = String.format("%d:%d", timestamp, fafUserAuthenticationToken.getUserId());

String token = Base64.getEncoder().encodeToString(new HmacUtils(HmacAlgorithms.HMAC_SHA_1, coturnServer.getKey()).hmac(tokenName));

String host = coturnServer.getHost();
if (coturnServer.getPort() != null) {
host += ":" + coturnServer.getPort();
}

Set<URI> urls = new HashSet<>();
urls.add(URI.create("turn://%s?transport=tcp".formatted(host)));
urls.add(URI.create("turn://%s?transport=udp".formatted(host)));
urls.add(URI.create("turn://%s".formatted(host)));

coturnServer.setUrls(urls);
coturnServer.setCredentialType("token");
coturnServer.setCredential(token);
coturnServer.setUsername(tokenName);
});
}
}
11 changes: 5 additions & 6 deletions src/main/java/com/faforever/api/data/DataController.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import org.springframework.http.HttpHeaders;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.security.access.annotation.Secured;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.security.core.Authentication;
import org.springframework.stereotype.Component;
Expand Down Expand Up @@ -41,7 +40,6 @@
*/
@RestController
@RequestMapping(path = DataController.PATH_PREFIX)
@Secured("ROLE_USER")
public class DataController {

public static final String PATH_PREFIX = "/data";
Expand All @@ -53,13 +51,10 @@ public DataController(JsonApi jsonApi) {
this.jsonApi = jsonApi;
}

private static User getPrincipal(final Authentication authentication) {
return new ElideUser(authentication);
}

//!!! No @Transactional - transactions are being handled by Elide
@GetMapping(value = {"/{entity}", "/{entity}/**"}, produces = JSON_API_MEDIA_TYPE)
@Cacheable(cacheResolver = "elideCacheResolver", keyGenerator = GetCacheKeyGenerator.NAME)
@PreAuthorize("isAuthenticated()")
public ResponseEntity<String> get(
@RequestParam final MultiValueMap<String, String> allRequestParams,
final HttpServletRequest request,
Expand Down Expand Up @@ -180,6 +175,10 @@ public ResponseEntity<String> delete(
return wrapResponse(response);
}

private static User getPrincipal(final Authentication authentication) {
return new ElideUser(authentication);
}

private ResponseEntity<String> wrapResponse(ElideResponse<String> response) {
return ResponseEntity.status(response.getStatus()).body(response.getBody());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public static class Inline extends OperationCheck<ClanMembership> {
@Override
public boolean ok(ClanMembership membership, RequestScope requestScope, Optional<ChangeSpec> changeSpec) {
final ElideUser caller = (ElideUser) requestScope.getUser();
final Integer requesterId = caller.getFafId().orElse(null);
final Integer requesterId = caller.getFafUserId().orElse(null);
return !Objects.equals(membership.getPlayer().getId(), membership.getClan().getLeader().getId())
&& (membership.getClan().getLeader().getId().equals(requesterId) ||
membership.getPlayer().getId().equals(requesterId));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public static class Inline extends OperationCheck<OwnableEntity> {
public boolean ok(OwnableEntity entity, RequestScope requestScope, Optional<ChangeSpec> changeSpec) {
final ElideUser caller = (ElideUser) requestScope.getUser();
return entity.getEntityOwner() != null
&& entity.getEntityOwner().getId().equals(caller.getFafId().orElse(null));
&& entity.getEntityOwner().getId().equals(caller.getFafUserId().orElse(null));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public static class Filter extends FilterExpressionCheck<OwnableEntity> {
@Override
public FilterExpression getFilterExpression(Type<?> entityClass, RequestScope requestScope) {
final ElideUser caller = (ElideUser) requestScope.getUser();
final int playerId = caller.getFafId().orElse(NON_EXISTING_PLAYER_ID);
final int playerId = caller.getFafUserId().orElse(NON_EXISTING_PLAYER_ID);
List<PathElement> pathList = new ArrayList<>();
getOwnerPath(entityClass, pathList);
Path path = new Path(pathList);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public class GroupPermission extends AbstractEntity<GroupPermission> implements
public static final String ROLE_ADMIN_MAP = "ADMIN_MAP";
public static final String ROLE_ADMIN_MOD = "ADMIN_MOD";
public static final String ROLE_WRITE_MESSAGE = "WRITE_MESSAGE";
public static final String ROLE_USER = "USER";

private String technicalName;
private String nameKey;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ public class BanInfoCreateHook implements LifeCycleHook<BanInfo> {
@Override
public void execute(Operation operation, TransactionPhase phase, BanInfo banInfo, RequestScope requestScope, Optional<ChangeSpec> changes) {
final ElideUser caller = (ElideUser) requestScope.getUser();
caller.getFafId().ifPresent(fafId -> {
caller.getFafUserId().ifPresent(playerId -> {
final Player callerPlayer = new Player();
callerPlayer.setId(fafId);
callerPlayer.setId(playerId);
banInfo.setAuthor(callerPlayer);
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ public class BanInfoRevokeAuthorUpdateHook implements LifeCycleHook<BanInfo> {
@Override
public void execute(Operation operation, TransactionPhase phase, BanInfo banInfo, RequestScope requestScope, Optional<ChangeSpec> changes) {
final ElideUser caller = (ElideUser) requestScope.getUser();
caller.getFafId().ifPresent(fafId -> {
caller.getFafUserId().ifPresent(playerId -> {
final Player callerPlayer = new Player();
callerPlayer.setId(fafId);
callerPlayer.setId(playerId);
banInfo.setRevokeAuthor(callerPlayer);
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ public class ModerationReportHook implements LifeCycleHook<ModerationReport> {
@Override
public void execute(Operation operation, TransactionPhase phase, ModerationReport moderationReport, RequestScope requestScope, Optional<ChangeSpec> changes) {
final ElideUser caller = (ElideUser) requestScope.getUser();
final Player callerPlayer = caller.getFafId().map(fafId -> {
final Player callerPlayer = caller.getFafUserId().map(fafPlayerId -> {
final Player player = new Player();
player.setId(fafId);
player.setId(fafPlayerId);
return player;
}).orElse(null);
if (operation == Operation.CREATE) {
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/com/faforever/api/player/PlayerService.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import com.faforever.api.error.ApiException;
import com.faforever.api.error.Error;
import com.faforever.api.error.ErrorCode;
import com.faforever.api.security.FafAuthenticationToken;
import com.faforever.api.security.FafUserAuthenticationToken;
import lombok.RequiredArgsConstructor;
import org.springframework.security.core.Authentication;
import org.springframework.stereotype.Service;
Expand All @@ -18,8 +18,8 @@ public class PlayerService {
private final PlayerRepository playerRepository;

public Player getPlayer(Authentication authentication) {
if (authentication instanceof FafAuthenticationToken fafAuthenticationToken) {
return playerRepository.findById((fafAuthenticationToken.getUserId()))
if (authentication instanceof FafUserAuthenticationToken fafUserAuthenticationToken) {
return playerRepository.findById((fafUserAuthenticationToken.getUserId()))
.orElseThrow(() -> new ApiException(new Error(TOKEN_INVALID)));
}
throw new ApiException(new Error(TOKEN_INVALID));
Expand Down
19 changes: 15 additions & 4 deletions src/main/java/com/faforever/api/security/AuditService.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,27 @@
@Service
public class AuditService {

private UserSupplier userSupplier;
private final UserSupplier userSupplier;

public AuditService(UserSupplier userSupplier) {
this.userSupplier = userSupplier;
}

public void logMessage(String message) {
final FafAuthenticationToken fafAuthenticationToken = userSupplier.get();
String extendedMessage = MessageFormat.format("{0} [invoked by User ''{1}'' with id ''{2}'']",
message, fafAuthenticationToken.getUsername(), fafAuthenticationToken.getUserId());
final String extendedMessage = userSupplier.get()
.map(fafAuthenticationToken -> {
//move to switch pattern matching with java 21
if (fafAuthenticationToken instanceof FafUserAuthenticationToken fafUserAuthenticationToken) {
return MessageFormat.format("{0} [invoked by User ''{1}'' with id ''{2}'']",
message, fafUserAuthenticationToken.getUsername(), fafUserAuthenticationToken.getUserId());
} else if (fafAuthenticationToken instanceof FafServiceAuthenticationToken fafServiceAuthenticationToken) {
return MessageFormat.format("{0} [invoked by Service ''{1}'']",
message, fafServiceAuthenticationToken.getServiceName());

Check warning on line 27 in src/main/java/com/faforever/api/security/AuditService.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/faforever/api/security/AuditService.java#L26-L27

Added lines #L26 - L27 were not covered by tests
} else {
throw new RuntimeException();

Check warning on line 29 in src/main/java/com/faforever/api/security/AuditService.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/faforever/api/security/AuditService.java#L29

Added line #L29 was not covered by tests
}
})
.orElseGet(() -> MessageFormat.format("{0} [invoked by Annonymous user]", message));
log.info(extendedMessage);
}
}
7 changes: 5 additions & 2 deletions src/main/java/com/faforever/api/security/ElideUser.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ public boolean isInRole(String role) {
return fafAuthentication.hasRole(role);
}

public Optional<Integer> getFafId() {
return Optional.ofNullable(fafAuthentication).map(FafAuthenticationToken::getUserId);
public Optional<Integer> getFafUserId() {
return Optional.ofNullable(fafAuthentication)
.filter(o -> FafUserAuthenticationToken.class.isAssignableFrom(o.getClass()))
.map(FafUserAuthenticationToken.class::cast)
.map(FafUserAuthenticationToken::getUserId);
}
}
Original file line number Diff line number Diff line change
@@ -1,29 +1,35 @@
package com.faforever.api.security;

import java.util.List;
import java.util.Map;

import org.springframework.core.convert.converter.Converter;
import org.springframework.security.authentication.AbstractAuthenticationToken;
import org.springframework.security.oauth2.jwt.Jwt;

import java.util.List;
import java.util.Map;

/**
* Jwt converter that reads scopes + custom FAF roles from the token extension.
*/
public class FafAuthenticationConverter implements Converter<Jwt, AbstractAuthenticationToken> {

@Override
public AbstractAuthenticationToken convert(Jwt source) {
int userId = extractUserId(source);
String username = extractUsername(source);
List<FafScope> scopes = extractScopes(source);
List<FafRole> roles = extractRoles(source);

return new FafAuthenticationToken(userId, username, scopes, roles);
String subject = extractSubject(source);

Check warning on line 20 in src/main/java/com/faforever/api/security/FafAuthenticationConverter.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/faforever/api/security/FafAuthenticationConverter.java#L20

Added line #L20 was not covered by tests

try {
int userId = Integer.parseInt(subject);
String username = extractUsername(source);
return new FafUserAuthenticationToken(userId, username, scopes, roles);
} catch (NumberFormatException e) {
return new FafServiceAuthenticationToken(subject, scopes);

Check warning on line 27 in src/main/java/com/faforever/api/security/FafAuthenticationConverter.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/faforever/api/security/FafAuthenticationConverter.java#L23-L27

Added lines #L23 - L27 were not covered by tests
}
}

private int extractUserId(Jwt source) {
return Integer.parseInt(source.getSubject());
private String extractSubject(Jwt source) {
return source.getSubject();

Check warning on line 32 in src/main/java/com/faforever/api/security/FafAuthenticationConverter.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/faforever/api/security/FafAuthenticationConverter.java#L32

Added line #L32 was not covered by tests
}

private String extractUsername(Jwt source) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,22 @@
import java.util.Collection;

@Getter
public class FafAuthenticationToken extends AbstractAuthenticationToken {
public abstract sealed class FafAuthenticationToken extends AbstractAuthenticationToken
permits FafUserAuthenticationToken, FafServiceAuthenticationToken {

private final int userId;
private final String username;
private final Collection<FafScope> scopes;
private final Collection<FafRole> roles;
protected final Collection<FafScope> scopes;
protected final Collection<FafRole> roles;

public FafAuthenticationToken(
int userId,
String username,
@NotNull Collection<FafScope> scopes,
@NotNull Collection<FafRole> roles
) {
super(
ImmutableList.<GrantedAuthority>builder()
.addAll(scopes)
.addAll(roles)
// ROLE_USER is an implicit role by Spring Security usually set during regular authentication, so we add it too
.add((GrantedAuthority) () -> "ROLE_USER")
.build()
);
this.userId = userId;
this.username = username;
this.scopes = scopes;
this.roles = roles;
// since the access token was already verified, each FafAuthenticationToken is implicitly authenticated
Expand All @@ -43,16 +36,6 @@ public Object getCredentials() {
return null;
}

@Override
public Object getPrincipal() {
return userId;
}

@Override
public String getName() {
return String.format("User %s", userId);
}

public boolean hasRole(String role) {
return roles.stream()
.anyMatch(r -> r.matches(role));
Expand Down
Loading

0 comments on commit 4828ec1

Please sign in to comment.