Skip to content

Commit 17de181

Browse files
committed
refactor: centralize repeated string constants
1 parent 9677de3 commit 17de181

File tree

3 files changed

+49
-21
lines changed

3 files changed

+49
-21
lines changed
Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,31 @@
11
package io.github.fmfi_svt.andrvotr;
22

33
public final class Constants {
4-
public static final String AUTHORITY_TOKEN_INNER_PREFIX = "ANDRVOTR_AUTHORITY_TOKEN_V1";
4+
private Constants() {}
55

6+
// Strings used in Andrvotr Authority Tokens. Produced by AuthorityTokenGenerator and consumed by HttpController.
7+
public static final String AUTHORITY_TOKEN_INNER_PREFIX = "ANDRVOTR_AUTHORITY_TOKEN_V1";
68
public static final String AUTHORITY_TOKEN_OUTER_PREFIX = "A1:";
9+
10+
// HTTP header names used for internal communication between HttpController and FabricationWebflowListener.
11+
public static final String HEADER_ANDRVOTR_INTERNAL_FABRICATION_TOKEN = "Andrvotr-Internal-Fabrication-Token";
12+
public static final String HEADER_ANDRVOTR_INTERNAL_FABRICATION_FRONT = "Andrvotr-Internal-Fabrication-Front";
13+
public static final String HEADER_ANDRVOTR_INTERNAL_FABRICATION_TRACE = "Andrvotr-Internal-Fabrication-Trace";
14+
15+
// Token value used for internal communication between HttpController and FabricationWebflowListener.
16+
public static final String ANDRVOTR_FABRICATION_TOKEN_VALUE = "andrvotr-fabrication-token";
17+
18+
// State and event names defined in the Shibboleth flow "SAML2/Redirect/SSO". Arguably an internal implementation
19+
// detail of Shibboleth. See class doc of FabricationWebflowListener.
20+
public static final String STATE_DECODE_MESSAGE = "DecodeMessage";
21+
public static final String STATE_HANDLE_OUTBOUND_MESSAGE = "HandleOutboundMessage";
22+
public static final String STATE_END = "end";
23+
public static final String EVENT_PROCEED = "proceed";
24+
25+
// Pseudo state names written in the fabrication trace. Used for internal communication between HttpController and
26+
// FabricationWebflowListener, and occasionally returned to the client on errors.
27+
public static final String TRACE_START = "@Start";
28+
public static final String TRACE_ALLOWED_CONNECTION_CHECK = "@AllowedConnectionCheck";
29+
public static final String TRACE_ALLOWED_CONNECTION_CHECK_SUCCESS = "@AllowedConnectionCheckSuccess";
30+
public static final String TRACE_ALLOWED_CONNECTION_CHECK_FAILURE = "@AllowedConnectionCheckFailure";
731
}

andrvotr-impl/src/main/java/io/github/fmfi_svt/andrvotr/FabricationWebflowListener.java

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737
/// java-identity-provider/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/saml/saml-abstract-flow.xml.
3838
public final class FabricationWebflowListener extends AbstractInitializableComponent implements FlowExecutionListener {
3939

40+
private static final String ANDRVOTR_FABRICATION_TOKEN_OK = "andrvotr_fabrication_token_ok";
41+
4042
private final @Nonnull Logger log = LoggerFactory.getLogger(FabricationWebflowListener.class);
4143

4244
private Config config;
@@ -71,7 +73,7 @@ public void requestSubmitted(RequestContext context) {
7173
(HttpServletRequest) context.getExternalContext().getNativeRequest();
7274

7375
// If the request does not have the Andrvotr-Internal-Fabrication-Token header, do nothing.
74-
String token = request.getHeader("Andrvotr-Internal-Fabrication-Token");
76+
String token = request.getHeader(Constants.HEADER_ANDRVOTR_INTERNAL_FABRICATION_TOKEN);
7577
if (token == null) {
7678
log.debug("no Andrvotr-Internal-Fabrication-Token - ignoring request");
7779
return;
@@ -82,7 +84,7 @@ public void requestSubmitted(RequestContext context) {
8284
// info in Andrvotr-Internal-Fabrication-Trace. But just in case.)
8385
try {
8486
String content = dataSealer.unwrap(token);
85-
if (!"andrvotr-fabrication-token".equals(content)) {
87+
if (!Constants.ANDRVOTR_FABRICATION_TOKEN_VALUE.equals(content)) {
8688
throw new Exception("wrong unwrapped value");
8789
}
8890
} catch (Exception e) {
@@ -93,8 +95,8 @@ public void requestSubmitted(RequestContext context) {
9395
}
9496

9597
log.info("started {} as a nested request inside andrvotr/fabricate", request.getRequestURI());
96-
context.getRequestScope().put("andrvotr_fabrication_token_ok", new Object());
97-
addTrace(context, "@Start");
98+
context.getRequestScope().put(ANDRVOTR_FABRICATION_TOKEN_OK, new Object());
99+
addTrace(context, Constants.TRACE_START);
98100
}
99101

100102
@Override
@@ -103,15 +105,16 @@ public void eventSignaled(RequestContext context, Event event) {
103105
(HttpServletRequest) context.getExternalContext().getNativeRequest();
104106

105107
// If the request does not have the Andrvotr-Internal-Fabrication-Token header, do nothing.
106-
if (!context.getRequestScope().contains("andrvotr_fabrication_token_ok")) return;
108+
if (!context.getRequestScope().contains(ANDRVOTR_FABRICATION_TOKEN_OK)) return;
107109

108110
// If we're leaving the "DecodeMessage" state with the "proceed" event (not an error), check whether our
109111
// configuration allows connections from the front entity ID (sent by HttpController in a header) to the back
110112
// entity ID (found in the decoded SAML message).
111-
if ("DecodeMessage".equals(context.getCurrentState().getId()) && "proceed".equals(event.getId())) {
112-
addTrace(context, "@AllowedConnectionCheck");
113+
if (Constants.STATE_DECODE_MESSAGE.equals(context.getCurrentState().getId())
114+
&& Constants.EVENT_PROCEED.equals(event.getId())) {
115+
addTrace(context, Constants.TRACE_ALLOWED_CONNECTION_CHECK);
113116

114-
String frontID = request.getHeader("Andrvotr-Internal-Fabrication-Front");
117+
String frontID = request.getHeader(Constants.HEADER_ANDRVOTR_INTERNAL_FABRICATION_FRONT);
115118

116119
// RelyingPartyContext is created by the "InitializeRelyingPartyContextFromSAMLPeer" action which runs
117120
// during the "DecodeMessage" state.
@@ -123,23 +126,23 @@ public void eventSignaled(RequestContext context, Event event) {
123126
|| Strings.isNullOrEmpty(backID)
124127
|| !config.isAllowedConnection(frontID, backID)) {
125128
log.error("forbidden andrvotr connection: front={} back={}", frontID, backID);
126-
addTrace(context, "@AllowedConnectionCheckFail");
129+
addTrace(context, Constants.TRACE_ALLOWED_CONNECTION_CHECK_FAILURE);
127130
throw new RuntimeException("Andrvotr fabricate failed - this connection is not allowed");
128131
}
129132

130133
log.info("allowed andrvotr connection: front={} back={}", frontID, backID);
131-
addTrace(context, "@AllowedConnectionCheckSuccess");
134+
addTrace(context, Constants.TRACE_ALLOWED_CONNECTION_CHECK_SUCCESS);
132135
}
133136
}
134137

135138
@Override
136139
public void stateEntered(RequestContext context, StateDefinition previousState, StateDefinition state) {
137140
// If the request does not have the Andrvotr-Internal-Fabrication-Token header, do nothing.
138-
if (!context.getRequestScope().contains("andrvotr_fabrication_token_ok")) return;
141+
if (!context.getRequestScope().contains(ANDRVOTR_FABRICATION_TOKEN_OK)) return;
139142

140143
// When moving from "HandleOutboundMessage" to "end", it is expected that the response is already sent, and we
141144
// can't add response headers anymore. Avoid the warning in addTrace.
142-
if ("end".equals(state.getId())) return;
145+
if (Constants.STATE_END.equals(state.getId())) return;
143146

144147
// Save all entered states in a response header for troubleshooting.
145148
addTrace(context, state.getId());
@@ -151,7 +154,7 @@ private void addTrace(RequestContext context, String value) {
151154

152155
if (!response.isCommitted()) {
153156
log.debug("adding Andrvotr-Internal-Fabrication-Trace: {}", value);
154-
response.addHeader("Andrvotr-Internal-Fabrication-Trace", value);
157+
response.addHeader(Constants.HEADER_ANDRVOTR_INTERNAL_FABRICATION_TRACE, value);
155158
} else {
156159
log.warn("response already committed, cannot add trace '{}'", value);
157160
}

andrvotr-impl/src/main/java/io/github/fmfi_svt/andrvotr/HttpController.java

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ public void fabricate(@Nonnull HttpServletRequest httpRequest, @Nonnull HttpServ
163163
try {
164164
// Expiration just for the sake of it. The exact length doesn't really matter.
165165
Instant expiration = Instant.now().plus(Duration.ofMinutes(10));
166-
fabricationToken = dataSealer.wrap("andrvotr-fabrication-token", expiration);
166+
fabricationToken = dataSealer.wrap(Constants.ANDRVOTR_FABRICATION_TOKEN_VALUE, expiration);
167167
} catch (Exception e) {
168168
log.error("DataSealer.wrap failed", e);
169169
sendError(httpResponse, 500, "DataSealer.wrap failed");
@@ -172,15 +172,16 @@ public void fabricate(@Nonnull HttpServletRequest httpRequest, @Nonnull HttpServ
172172

173173
HttpGet nestedRequest = new HttpGet(targetUrl);
174174
nestedRequest.addHeader("Cookie", cookies);
175-
nestedRequest.addHeader("Andrvotr-Internal-Fabrication-Token", fabricationToken);
176-
nestedRequest.addHeader("Andrvotr-Internal-Fabrication-Front", frontEntityID);
175+
nestedRequest.addHeader(Constants.HEADER_ANDRVOTR_INTERNAL_FABRICATION_TOKEN, fabricationToken);
176+
nestedRequest.addHeader(Constants.HEADER_ANDRVOTR_INTERNAL_FABRICATION_FRONT, frontEntityID);
177177

178178
httpClient.execute(nestedRequest, (nestedResponse) -> {
179179
int statusCode = nestedResponse.getCode();
180180
String contentType = nestedResponse.getEntity().getContentType();
181181
long contentLength = nestedResponse.getEntity().getContentLength();
182182

183-
List<String> trace = Arrays.stream(nestedResponse.getHeaders("Andrvotr-Internal-Fabrication-Trace"))
183+
List<String> trace = Arrays.stream(
184+
nestedResponse.getHeaders(Constants.HEADER_ANDRVOTR_INTERNAL_FABRICATION_TRACE))
184185
.map(Header::getValue)
185186
.collect(Collectors.toList());
186187

@@ -193,9 +194,9 @@ public void fabricate(@Nonnull HttpServletRequest httpRequest, @Nonnull HttpServ
193194
&& contentType != null
194195
&& contentType.startsWith("text/html")
195196
&& !trace.isEmpty()
196-
&& "@Start".equals(trace.get(0))
197-
&& trace.contains("@AllowedConnectionCheckSuccess")
198-
&& "HandleOutboundMessage".equals(trace.get(trace.size() - 1));
197+
&& Constants.TRACE_START.equals(trace.get(0))
198+
&& trace.contains(Constants.TRACE_ALLOWED_CONNECTION_CHECK_SUCCESS)
199+
&& Constants.STATE_HANDLE_OUTBOUND_MESSAGE.equals(trace.get(trace.size() - 1));
199200

200201
if (!success) {
201202
String message = String.format(

0 commit comments

Comments
 (0)