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

Revert retry verification and bypass cache #246

Merged
merged 2 commits into from
Dec 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ dependencies {
implementation 'org.springframework.boot:spring-boot-starter-web'
implementation 'org.springframework.boot:spring-boot-starter-security'
implementation 'org.springframework.boot:spring-boot-starter-validation'
implementation 'org.springframework.boot:spring-boot-starter-aop'
implementation 'org.springframework.retry:spring-retry'
implementation 'org.keycloak:keycloak-spring-boot-starter'
implementation "org.hibernate:hibernate-envers"
implementation 'org.hibernate:hibernate-spatial'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@
import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.builder.SpringApplicationBuilder;
import org.springframework.retry.annotation.EnableRetry;

/**
* Main application.
*/
@SpringBootApplication
@EnableRetry
public class BannergressBackendApplication {
public static void main(String[] args) {
SpringApplication application = new SpringApplicationBuilder(BannergressBackendApplication.class).build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import com.bannergress.backend.dto.UserDto;
import com.bannergress.backend.entities.User;
import com.bannergress.backend.exceptions.VerificationFailedException;
import com.bannergress.backend.exceptions.VerificationStateException;
import com.bannergress.backend.services.AgentService;
import com.bannergress.backend.services.UserMappingService;
Expand Down Expand Up @@ -57,7 +56,7 @@ public UserDto clearClaim(Principal principal) {
}

@PostMapping("/user/verify")
public UserDto verify(Principal principal) throws VerificationStateException, VerificationFailedException {
public UserDto verify(Principal principal) throws VerificationStateException {
String userId = principal.getName();
userService.verify(userId);
return get(principal);
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import org.springframework.web.bind.annotation.ResponseStatus;

/**
* Exception for when a verification fails because the user has not started the verification process yet.
* Exception for when a verification fails.
*/
@ResponseStatus(HttpStatus.PRECONDITION_FAILED)
public class VerificationStateException extends Exception {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package com.bannergress.backend.services;

import com.bannergress.backend.entities.User;
import com.bannergress.backend.exceptions.VerificationFailedException;
import com.bannergress.backend.exceptions.VerificationStateException;

import java.util.Optional;

/**
* Service for user-related tasks.
*/
Expand All @@ -28,10 +29,10 @@ public interface UserService {
* Verifies a user.
*
* @param userId user to verify.
* @return If the verification was successful: agent name, corrected for case sensitivity.
* @throws VerificationStateException If the verification process hasn't been started.
* @throws VerificationFailedException If the verification process fails.
*/
public void verify(String userId) throws VerificationStateException, VerificationFailedException;
public Optional<String> verify(String userId) throws VerificationStateException;

/**
* Unlinks a user from an agent name.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.bannergress.backend.services;

import com.bannergress.backend.exceptions.VerificationFailedException;

import java.util.Optional;
import java.util.UUID;

/**
Expand All @@ -13,8 +12,7 @@ public interface VerificationService {
*
* @param agent Agent name to verify control.
* @param verificationToken Token for agent verification.
* @return agent name, corrected for case sensitivity.
* @throws VerificationFailedException If the control over the agent name could not be established.
* @return If the verification was successful: agent name, corrected for case sensitivity.
*/
String verify(String agent, UUID verificationToken) throws VerificationFailedException;
Optional<String> verify(String agent, UUID verificationToken);
}
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
package com.bannergress.backend.services.impl;

import com.bannergress.backend.dto.RSS;
import com.bannergress.backend.exceptions.VerificationFailedException;
import com.bannergress.backend.services.VerificationService;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.dataformat.xml.XmlMapper;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.Response;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.retry.annotation.Backoff;
import org.springframework.retry.annotation.Retryable;
import org.springframework.http.HttpHeaders;
import org.springframework.stereotype.Service;

import java.io.IOException;
Expand All @@ -23,21 +21,24 @@ public class CommunityForumVerificationServiceImpl implements VerificationServic

private final String url;

private final String cacheBypassCookieHeader;

public CommunityForumVerificationServiceImpl(
@Value(value = "${verification.url:https://community.ingress.com/en/activity/feed.rss}") String url) {
@Value(value = "${verification.url:https://community.ingress.com/en/activity/feed.rss}") String url,
@Value(value = "${verification.cookie:vfo_s=dummy}") String cacheBypassCookieHeader) {
this.client = new OkHttpClient.Builder().cache(null).build();
this.url = url;
this.cacheBypassCookieHeader = cacheBypassCookieHeader;
}

@Override
@Retryable(include = VerificationFailedException.class, maxAttemptsExpression = "${verification.maxAttempts:3}", backoff = @Backoff(delayExpression = "${verification.backoff:60000}"))
public String verify(String agent, UUID verificationToken) throws VerificationFailedException {
public Optional<String> verify(String agent, UUID verificationToken) {
RSS rss = loadRssFeed();
return verify(agent, verificationToken, rss).orElseThrow(VerificationFailedException::new);
return verify(agent, verificationToken, rss);
}

private RSS loadRssFeed() {
Request request = new Request.Builder().url(url).build();
Request request = new Request.Builder().url(url).header(HttpHeaders.COOKIE, cacheBypassCookieHeader).build();
try (Response response = client.newCall(request).execute()) {
XmlMapper xmlMapper = new XmlMapper();
xmlMapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.bannergress.backend.services.impl;

import com.bannergress.backend.entities.User;
import com.bannergress.backend.exceptions.VerificationFailedException;
import com.bannergress.backend.exceptions.VerificationStateException;
import com.bannergress.backend.repositories.UserRepository;
import com.bannergress.backend.services.UserMappingService;
Expand Down Expand Up @@ -45,14 +44,20 @@ public void claim(String userId, String agent) {
}

@Override
public void verify(String userId) throws VerificationStateException, VerificationFailedException {
public Optional<String> verify(String userId) throws VerificationStateException {
User user = getOrCreate(userId);
if (user.getVerificationAgent() == null || user.getVerificationToken() == null) {
throw new VerificationStateException();
}
String agentName = verificationService.verify(user.getVerificationAgent(), user.getVerificationToken());
userMappingService.setAgentName(userId, agentName);
clearClaim(userId);
Optional<String> agentName = verificationService.verify(user.getVerificationAgent(),
user.getVerificationToken());
if (agentName.isPresent()) {
userMappingService.setAgentName(userId, agentName.get());
clearClaim(userId);
} else {
throw new VerificationStateException();
}
return agentName;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
package com.bannergress.backend.services.impl;

import com.bannergress.backend.exceptions.VerificationFailedException;
import com.bannergress.backend.services.VerificationService;
import org.junit.jupiter.api.Test;

import java.util.Optional;
import java.util.UUID;

import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThat;

public class TestVerificationServiceImpl {
@Test
public void testCompletion() {
VerificationService verificationService = new CommunityForumVerificationServiceImpl(
"https://community.ingress.com/en/activity/feed.rss");
assertThatExceptionOfType(VerificationFailedException.class)
.isThrownBy(() -> verificationService.verify("someone", UUID.randomUUID()));
"https://community.ingress.com/en/activity/feed.rss", "vfo_s=dummy");
Optional<String> agent = verificationService.verify("someone", UUID.randomUUID());
assertThat(agent).isEmpty();
}
}