Skip to content

Commit 76f011c

Browse files
shs96cshs96c
committed
[grid] Rework secret checks
1 parent 7c7e296 commit 76f011c

27 files changed

Lines changed: 218 additions & 112 deletions

‎java/server/src/org/openqa/selenium/grid/distributor/AddNode.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,19 @@ class AddNode implements HttpHandler {
3939
private final Distributor distributor;
4040
private final Json json;
4141
private final HttpClient.Factory httpFactory;
42+
private final String registrationSecret;
4243

4344
AddNode(
4445
Tracer tracer,
4546
Distributor distributor,
4647
Json json,
47-
HttpClient.Factory httpFactory) {
48+
HttpClient.Factory httpFactory,
49+
String registrationSecret) {
4850
this.tracer = Require.nonNull("Tracer", tracer);
4951
this.distributor = Require.nonNull("Distributor", distributor);
5052
this.json = Require.nonNull("Json converter", json);
5153
this.httpFactory = Require.nonNull("HTTP Factory", httpFactory);
54+
this.registrationSecret = registrationSecret;
5255
}
5356

5457
@Override
@@ -60,6 +63,7 @@ public HttpResponse execute(HttpRequest req) {
6063
httpFactory,
6164
status.getNodeId(),
6265
status.getUri(),
66+
registrationSecret,
6367
status.getSlots().stream().map(Slot::getStereotype).collect(Collectors.toSet()));
6468

6569
distributor.add(node);

‎java/server/src/org/openqa/selenium/grid/distributor/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ java_library(
2020
"//java/server/src/org/openqa/selenium/grid/distributor/selector",
2121
"//java/server/src/org/openqa/selenium/grid/node",
2222
"//java/server/src/org/openqa/selenium/grid/node/remote",
23+
"//java/server/src/org/openqa/selenium/grid/security",
2324
"//java/server/src/org/openqa/selenium/grid/sessionmap",
2425
# Default implementation of the session map. Loaded reflectively
2526
"//java/server/src/org/openqa/selenium/grid/sessionmap/remote",

‎java/server/src/org/openqa/selenium/grid/distributor/Distributor.java‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.openqa.selenium.grid.distributor.model.Host;
3232
import org.openqa.selenium.grid.distributor.selector.SlotSelector;
3333
import org.openqa.selenium.grid.node.Node;
34+
import org.openqa.selenium.grid.security.RequiresSecretFilter;
3435
import org.openqa.selenium.grid.sessionmap.SessionMap;
3536
import org.openqa.selenium.internal.Require;
3637
import org.openqa.selenium.json.Json;
@@ -129,26 +130,33 @@ protected Distributor(
129130
Tracer tracer,
130131
HttpClient.Factory httpClientFactory,
131132
SlotSelector slotSelector,
132-
SessionMap sessions) {
133+
SessionMap sessions,
134+
String registrationSecret) {
133135
this.tracer = Require.nonNull("Tracer", tracer);
134136
Require.nonNull("HTTP client factory", httpClientFactory);
135137
this.slotSelector = Require.nonNull("Host selector", slotSelector);
136138
this.sessions = Require.nonNull("Session map", sessions);
137139

140+
RequiresSecretFilter requiresSecret = new RequiresSecretFilter(registrationSecret);
141+
138142
Json json = new Json();
139143
routes = Route.combine(
140144
post("/session").to(() -> req -> {
141145
CreateSessionResponse sessionResponse = newSession(req);
142146
return new HttpResponse().setContent(bytes(sessionResponse.getDownstreamEncodedResponse()));
143147
}),
144148
post("/se/grid/distributor/session")
145-
.to(() -> new CreateSession(this)),
149+
.to(() -> new CreateSession(this))
150+
.with(requiresSecret),
146151
post("/se/grid/distributor/node")
147-
.to(() -> new AddNode(tracer, this, json, httpClientFactory)),
152+
.to(() -> new AddNode(tracer, this, json, httpClientFactory, registrationSecret))
153+
.with(requiresSecret),
148154
post("/se/grid/distributor/node/{nodeId}/drain")
149-
.to((Map<String, String> params) -> new DrainNode(this, new NodeId(UUID.fromString(params.get("nodeId"))))),
155+
.to((Map<String, String> params) -> new DrainNode(this, new NodeId(UUID.fromString(params.get("nodeId")))))
156+
.with(requiresSecret),
150157
delete("/se/grid/distributor/node/{nodeId}")
151-
.to(params -> new RemoveNode(this, new NodeId(UUID.fromString(params.get("nodeId"))))),
158+
.to(params -> new RemoveNode(this, new NodeId(UUID.fromString(params.get("nodeId")))))
159+
.with(requiresSecret),
152160
get("/se/grid/distributor/status")
153161
.to(() -> new GetDistributorStatus(this))
154162
.with(new SpanDecorator(tracer, req -> "distributor.status")));
@@ -289,6 +297,4 @@ public boolean matches(HttpRequest req) {
289297
public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
290298
return routes.execute(req);
291299
}
292-
293-
public abstract String getRegistrationSecret();
294300
}

‎java/server/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ public LocalDistributor(
9494
HttpClient.Factory clientFactory,
9595
SessionMap sessions,
9696
String registrationSecret) {
97-
super(tracer, clientFactory, new DefaultSlotSelector(), sessions);
97+
super(tracer, clientFactory, new DefaultSlotSelector(), sessions, registrationSecret);
9898
this.tracer = Require.nonNull("Tracer", tracer);
9999
this.bus = Require.nonNull("Event bus", bus);
100100
this.clientFactory = Require.nonNull("HTTP client factory", clientFactory);
@@ -168,6 +168,7 @@ private void refresh(NodeStatus status) {
168168
clientFactory,
169169
status.getNodeId(),
170170
status.getUri(),
171+
registrationSecret,
171172
status.getSlots().stream().map(Slot::getStereotype).collect(Collectors.toSet()));
172173
add(node, status);
173174
}
@@ -258,11 +259,6 @@ public DistributorStatus getStatus() {
258259
}
259260
}
260261

261-
@Override
262-
public String getRegistrationSecret() {
263-
return registrationSecret;
264-
}
265-
266262
@Beta
267263
public void refresh() {
268264
Lock writeLock = lock.writeLock();

‎java/server/src/org/openqa/selenium/grid/distributor/remote/RemoteDistributor.java‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,13 @@ public class RemoteDistributor extends Distributor {
5151
private static final Logger LOG = Logger.getLogger("Selenium Distributor (Remote)");
5252
private final HttpHandler client;
5353

54-
public RemoteDistributor(Tracer tracer, HttpClient.Factory factory, URL url) {
54+
public RemoteDistributor(Tracer tracer, HttpClient.Factory factory, URL url, String registrationSecret) {
5555
super(
5656
tracer,
5757
factory,
5858
(caps, nodes) -> {throw new UnsupportedOperationException("host selection");},
59-
new NullSessionMap(tracer));
59+
new NullSessionMap(tracer),
60+
registrationSecret);
6061
this.client = factory.createClient(url);
6162
}
6263

@@ -138,8 +139,4 @@ protected Set<Host> getModel() {
138139
protected Supplier<CreateSessionResponse> reserve(SlotId slot, CreateSessionRequest request) {
139140
throw new UnsupportedOperationException("reserve is not required for remote sessions");
140141
}
141-
142-
public String getRegistrationSecret() {
143-
return "";
144-
}
145142
}

‎java/server/src/org/openqa/selenium/grid/node/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ java_library(
1717
"//java/client/src/org/openqa/selenium/remote",
1818
"//java/server/src/org/openqa/selenium/grid/component",
1919
"//java/server/src/org/openqa/selenium/grid/data",
20+
"//java/server/src/org/openqa/selenium/grid/security",
2021
"//java/server/src/org/openqa/selenium/grid/web",
2122
"//java/server/src/org/openqa/selenium/status",
2223
artifact("com.google.guava:guava"),

‎java/server/src/org/openqa/selenium/grid/node/ForwardWebDriverCommand.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,6 @@ class ForwardWebDriverCommand implements HttpHandler {
3232

3333
@Override
3434
public HttpResponse execute(HttpRequest req) {
35-
return node.executeWebDriverCommand(req).setHeader("X-REGISTRATION-SECRET", node.getRegistrationSecret());
35+
return node.executeWebDriverCommand(req);
3636
}
3737
}

‎java/server/src/org/openqa/selenium/grid/node/GetNodeSession.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,6 @@ class GetNodeSession implements HttpHandler {
4343
public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
4444
Session session = node.getSession(id);
4545

46-
return new HttpResponse().setContent(asJson(ImmutableMap.of("value", session))).setHeader("X-REGISTRATION-SECRET", node.getRegistrationSecret());
46+
return new HttpResponse().setContent(asJson(ImmutableMap.of("value", session)));
4747
}
4848
}

‎java/server/src/org/openqa/selenium/grid/node/IsSessionOwner.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,6 @@ class IsSessionOwner implements HttpHandler {
4141

4242
@Override
4343
public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
44-
return new HttpResponse().setContent(asJson(ImmutableMap.of("value", node.isSessionOwner(id)))).setHeader("X-REGISTRATION-SECRET", node.getRegistrationSecret());
44+
return new HttpResponse().setContent(asJson(ImmutableMap.of("value", node.isSessionOwner(id))));
4545
}
4646
}

‎java/server/src/org/openqa/selenium/grid/node/NewNodeSession.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,6 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
5050
HashMap<String, Object> value = new HashMap<>();
5151
value.put("value", sessionResponse);
5252

53-
return new HttpResponse().setContent(asJson(value)).setHeader("X-REGISTRATION-SECRET", node.getRegistrationSecret());
53+
return new HttpResponse().setContent(asJson(value));
5454
}
5555
}

0 commit comments

Comments
 (0)