Skip to content

Commit fc45451

Browse files
MikeDomboMikeDombo
authored
Merge branch 'main' into list-dep
2 parents e45a361 + 57d53cd commit fc45451

File tree

9 files changed

+151
-22
lines changed

9 files changed

+151
-22
lines changed

‎.github/workflows/ci.yml

+6
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,12 @@ jobs:
2929
- name: Build with Maven (not Windows)
3030
run: mvn -U -ntp clean verify
3131
if: matrix.os != 'windows-latest'
32+
- name: Upload Failed Test Report
33+
uses: actions/[email protected]
34+
if: failure()
35+
with:
36+
name: Failed Test Report ${{ matrix.os }}
37+
path: server/target/surefire-reports
3238
- name: Convert Jacoco to Cobertura
3339
run: |
3440
python3 ./.github/scripts/cover2cover.py client/target/jacoco-report/jacoco.xml src/main/java > client/target/jacoco-report/cobertura.xml

‎client/pom.xml

+1-1
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@
8686
<dependency>
8787
<groupId>software.amazon.awssdk.iotdevicesdk</groupId>
8888
<artifactId>aws-iot-device-sdk</artifactId>
89-
<version>1.5.1-WINDOWS-SNAPSHOT</version>
89+
<version>1.9.2</version>
9090
</dependency>
9191
</dependencies>
9292
<build>

‎server/pom.xml

+1-1
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@
211211
<dependency>
212212
<groupId>com.aws.greengrass</groupId>
213213
<artifactId>nucleus</artifactId>
214-
<version>2.5.0-SNAPSHOT</version>
214+
<version>2.6.0-SNAPSHOT</version>
215215
<scope>provided</scope>
216216
</dependency>
217217
<dependency>

‎server/src/main/java/com/aws/greengrass/cli/CLIEventStreamAgent.java

+69-11
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55

66
package com.aws.greengrass.cli;
77

8+
import com.aws.greengrass.authorization.AuthorizationHandler;
9+
import com.aws.greengrass.authorization.Permission;
10+
import com.aws.greengrass.authorization.exceptions.AuthorizationException;
811
import com.aws.greengrass.componentmanager.ComponentStore;
912
import com.aws.greengrass.config.Node;
1013
import com.aws.greengrass.config.Topics;
@@ -96,6 +99,14 @@
9699
import static com.aws.greengrass.ipc.common.ExceptionUtil.translateExceptions;
97100
import static com.aws.greengrass.ipc.common.IPCErrorStrings.DEPLOYMENTS_QUEUE_FULL;
98101
import static com.aws.greengrass.ipc.common.IPCErrorStrings.DEPLOYMENTS_QUEUE_NOT_INITIALIZED;
102+
import static software.amazon.awssdk.aws.greengrass.GreengrassCoreIPCServiceModel.CREATE_DEBUG_PASSWORD;
103+
import static software.amazon.awssdk.aws.greengrass.GreengrassCoreIPCServiceModel.CREATE_LOCAL_DEPLOYMENT;
104+
import static software.amazon.awssdk.aws.greengrass.GreengrassCoreIPCServiceModel.GET_COMPONENT_DETAILS;
105+
import static software.amazon.awssdk.aws.greengrass.GreengrassCoreIPCServiceModel.GET_LOCAL_DEPLOYMENT_STATUS;
106+
import static software.amazon.awssdk.aws.greengrass.GreengrassCoreIPCServiceModel.LIST_COMPONENTS;
107+
import static software.amazon.awssdk.aws.greengrass.GreengrassCoreIPCServiceModel.LIST_LOCAL_DEPLOYMENTS;
108+
import static software.amazon.awssdk.aws.greengrass.GreengrassCoreIPCServiceModel.RESTART_COMPONENT;
109+
import static software.amazon.awssdk.aws.greengrass.GreengrassCoreIPCServiceModel.STOP_COMPONENT;
99110

100111
@SuppressWarnings("PMD.CouplingBetweenObjects")
101112
public class CLIEventStreamAgent {
@@ -119,6 +130,9 @@ public class CLIEventStreamAgent {
119130
@Setter(AccessLevel.PACKAGE)
120131
private DeploymentQueue deploymentQueue;
121132

133+
@Inject
134+
private AuthorizationHandler authHandler;
135+
122136
private final SecureRandom random = new SecureRandom();
123137

124138
public GetComponentDetailsHandler getGetComponentDetailsHandler(OperationContinuationHandlerContext context) {
@@ -190,8 +204,12 @@ protected void onStreamClosed() {
190204
@SuppressWarnings("PMD.PreserveStackTrace")
191205
public GetComponentDetailsResponse handleRequest(GetComponentDetailsRequest request) {
192206
return translateExceptions(() -> {
193-
authorizeRequest(componentName);
194207
validateGetComponentDetailsRequest(request);
208+
authorizeRequest(Permission.builder()
209+
.principal(componentName)
210+
.resource(request.getComponentName())
211+
.operation(GET_COMPONENT_DETAILS)
212+
.build());
195213
String componentName = request.getComponentName();
196214
GreengrassService service;
197215
try {
@@ -219,11 +237,23 @@ private void validateGetComponentDetailsRequest(GetComponentDetailsRequest reque
219237
}
220238
}
221239

222-
private void authorizeRequest(String componentName) {
223-
if (Utils.isEmpty(componentName) || !componentName.startsWith(GREENGRASS_CLI_CLIENT_ID_PREFIX) && !CLI_SERVICE
224-
.equals(componentName)) {
240+
private void authorizeRequest(Permission permission) {
241+
if (Utils.isEmpty(permission.getPrincipal())) {
225242
throw new UnauthorizedError("Component is not authorized to call CLI APIs");
226243
}
244+
// Allow CLI and CLI clients to call anything
245+
if (permission.getPrincipal().startsWith(GREENGRASS_CLI_CLIENT_ID_PREFIX)
246+
|| CLI_SERVICE.equals(permission.getPrincipal())) {
247+
return;
248+
}
249+
// Check for authorization for all other components
250+
try {
251+
authHandler.isAuthorized(CLI_SERVICE, permission);
252+
} catch (AuthorizationException e) {
253+
logger.atWarn().kv("error", e.getMessage()).kv("componentName", permission.getPrincipal())
254+
.log("Not Authorized");
255+
throw new UnauthorizedError(e.getMessage());
256+
}
227257
}
228258

229259
private ComponentDetails getComponentDetails(GreengrassService service) {
@@ -259,7 +289,11 @@ protected void onStreamClosed() {
259289
@Override
260290
public ListComponentsResponse handleRequest(ListComponentsRequest request) {
261291
return translateExceptions(() -> {
262-
authorizeRequest(componentName);
292+
authorizeRequest(Permission.builder()
293+
.principal(componentName)
294+
.resource(AuthorizationHandler.ANY_REGEX)
295+
.operation(LIST_COMPONENTS)
296+
.build());
263297
Collection<GreengrassService> services = kernel.orderedDependencies();
264298
List<ComponentDetails> listOfComponents =
265299
services.stream().filter(service -> service != kernel.getMain())
@@ -295,8 +329,12 @@ protected void onStreamClosed() {
295329
@SuppressWarnings("PMD.PreserveStackTrace")
296330
public RestartComponentResponse handleRequest(RestartComponentRequest request) {
297331
return translateExceptions(() -> {
298-
authorizeRequest(componentName);
299332
validateRestartComponentRequest(request);
333+
authorizeRequest(Permission.builder()
334+
.principal(componentName)
335+
.resource(request.getComponentName())
336+
.operation(RESTART_COMPONENT)
337+
.build());
300338
String componentName = request.getComponentName();
301339
RestartComponentResponse response = new RestartComponentResponse();
302340
response.setRestartStatus(RequestStatus.SUCCEEDED);
@@ -346,8 +384,12 @@ protected void onStreamClosed() {
346384
@SuppressWarnings("PMD.PreserveStackTrace")
347385
public StopComponentResponse handleRequest(StopComponentRequest request) {
348386
return translateExceptions(() -> {
349-
authorizeRequest(componentName);
350387
validateStopComponentRequest(request);
388+
authorizeRequest(Permission.builder()
389+
.principal(componentName)
390+
.resource(request.getComponentName())
391+
.operation(STOP_COMPONENT)
392+
.build());
351393
String componentName = request.getComponentName();
352394
try {
353395
GreengrassService service = kernel.locate(componentName);
@@ -396,7 +438,11 @@ protected void onStreamClosed() {
396438
@SuppressWarnings({"PMD.PreserveStackTrace", "PMD.AvoidCatchingGenericException"})
397439
public CreateLocalDeploymentResponse handleRequest(CreateLocalDeploymentRequest request) {
398440
return translateExceptions(() -> {
399-
authorizeRequest(componentName);
441+
authorizeRequest(Permission.builder()
442+
.principal(componentName)
443+
.resource(AuthorizationHandler.ANY_REGEX)
444+
.operation(CREATE_LOCAL_DEPLOYMENT)
445+
.build());
400446
String deploymentId = UUID.randomUUID().toString();
401447
//All inputs are valid. If all inputs are empty, then user might just want to retrigger the deployment
402448
// with new recipes set using the updateRecipesAndArtifacts API.
@@ -492,8 +538,12 @@ protected void onStreamClosed() {
492538
@Override
493539
public GetLocalDeploymentStatusResponse handleRequest(GetLocalDeploymentStatusRequest request) {
494540
return translateExceptions(() -> {
495-
authorizeRequest(componentName);
496541
validateGetLocalDeploymentStatusRequest(request);
542+
authorizeRequest(Permission.builder()
543+
.principal(componentName)
544+
.resource(request.getDeploymentId())
545+
.operation(GET_LOCAL_DEPLOYMENT_STATUS)
546+
.build());
497547
Topics localDeployments = cliServiceConfig.findTopics(PERSISTENT_LOCAL_DEPLOYMENTS);
498548
if (localDeployments == null || localDeployments.findTopics(request.getDeploymentId()) == null) {
499549
ResourceNotFoundError rnf = new ResourceNotFoundError();
@@ -558,7 +608,11 @@ public List<Node> sortDeploymentsByTime(Iterator<Node> deploymentsIterator, Comp
558608
@Override
559609
public ListLocalDeploymentsResponse handleRequest(ListLocalDeploymentsRequest request) {
560610
return translateExceptions(() -> {
561-
authorizeRequest(componentName);
611+
authorizeRequest(Permission.builder()
612+
.principal(componentName)
613+
.resource(AuthorizationHandler.ANY_REGEX)
614+
.operation(LIST_LOCAL_DEPLOYMENTS)
615+
.build());
562616
List<LocalDeployment> persistedDeployments = new ArrayList<>();
563617
Topics localDeployments = cliServiceConfig.findTopics(PERSISTENT_LOCAL_DEPLOYMENTS);
564618
List<Topics> deploymentsByTime = sortDeploymentsByTime(localDeployments.iterator(),
@@ -616,7 +670,11 @@ protected void onStreamClosed() {
616670
@Override
617671
public CreateDebugPasswordResponse handleRequest(CreateDebugPasswordRequest request) {
618672
return translateExceptions(() -> {
619-
authorizeRequest(componentName);
673+
authorizeRequest(Permission.builder()
674+
.principal(componentName)
675+
.resource(AuthorizationHandler.ANY_REGEX)
676+
.operation(CREATE_DEBUG_PASSWORD)
677+
.build());
620678
CreateDebugPasswordResponse response = new CreateDebugPasswordResponse();
621679

622680
String password = generatePassword(DEBUG_PASSWORD_LENGTH_REQUIREMENT);

‎server/src/main/java/com/aws/greengrass/cli/CLIService.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848

4949
@ImplementsService(name = CLIService.CLI_SERVICE, autostart = true)
5050
public class CLIService extends PluginService {
51-
public static final String GREENGRASS_CLI_CLIENT_ID_PREFIX = "greengrass-cli-";
51+
public static final String GREENGRASS_CLI_CLIENT_ID_PREFIX = "greengrass-cli#";
5252
public static final String GREENGRASS_CLI_CLIENT_ID_FMT = GREENGRASS_CLI_CLIENT_ID_PREFIX + "%s";
5353
public static final String CLI_SERVICE = "aws.greengrass.Cli";
5454
public static final String CLI_CLIENT_ARTIFACT = "aws.greengrass.cli.client";

‎server/src/test/java/com/aws/greengrass/cli/CLIEventStreamAgentTest.java

+25-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
package com.aws.greengrass.cli;
77

8+
import com.aws.greengrass.authorization.AuthorizationHandler;
9+
import com.aws.greengrass.authorization.exceptions.AuthorizationException;
810
import com.aws.greengrass.componentmanager.ComponentStore;
911
import com.aws.greengrass.config.Topics;
1012
import com.aws.greengrass.dependency.Context;
@@ -61,6 +63,7 @@
6163
import java.util.UUID;
6264

6365
import static com.aws.greengrass.cli.CLIEventStreamAgent.PERSISTENT_LOCAL_DEPLOYMENTS;
66+
import static com.aws.greengrass.cli.CLIService.CLI_SERVICE;
6467
import static com.aws.greengrass.cli.CLIService.GREENGRASS_CLI_CLIENT_ID_FMT;
6568
import static com.aws.greengrass.componentmanager.KernelConfigResolver.CONFIGURATION_CONFIG_KEY;
6669
import static com.aws.greengrass.componentmanager.KernelConfigResolver.VERSION_CONFIG_KEY;
@@ -76,6 +79,8 @@
7679
import static org.junit.jupiter.api.Assertions.assertTrue;
7780
import static org.junit.jupiter.api.Assertions.fail;
7881
import static org.mockito.ArgumentMatchers.any;
82+
import static org.mockito.ArgumentMatchers.eq;
83+
import static org.mockito.Mockito.lenient;
7984
import static org.mockito.Mockito.mock;
8085
import static org.mockito.Mockito.verify;
8186
import static org.mockito.Mockito.when;
@@ -101,27 +106,46 @@ class CLIEventStreamAgentTest {
101106
@Mock
102107
DeploymentQueue deploymentQueue;
103108

109+
@Mock
110+
AuthorizationHandler authorizationHandler;
111+
104112
@Mock
105113
private ComponentStore componentStore;
106114

107115
@InjectMocks
108116
private CLIEventStreamAgent cliEventStreamAgent;
109117

110118
@BeforeEach
111-
void setup() {
119+
void setup() throws AuthorizationException {
112120
when(mockContext.getContinuation()).thenReturn(mock(ServerConnectionContinuation.class));
113121
when(mockContext.getAuthenticationData()).thenReturn(() -> String.format(GREENGRASS_CLI_CLIENT_ID_FMT, "abc"));
122+
lenient().when(authorizationHandler.isAuthorized(eq(CLI_SERVICE), any()))
123+
.thenThrow(new AuthorizationException("bad"));
114124
}
115125

116126
@Test
117127
void test_GetComponentDetails_with_bad_auth_token_rejects() {
118128
// Pretend that TEST_SERVICE has connected and wants to call the CLI APIs
119129
when(mockContext.getAuthenticationData()).thenReturn(() -> TEST_SERVICE);
120130
GetComponentDetailsRequest request = new GetComponentDetailsRequest();
131+
request.setComponentName("any");
121132
assertThrows(UnauthorizedError.class,
122133
() -> cliEventStreamAgent.getGetComponentDetailsHandler(mockContext).handleRequest(request));
123134
}
124135

136+
@Test
137+
void test_GetComponentDetails_with_non_cli_auth_token_calls_authZ()
138+
throws AuthorizationException, ServiceLoadException {
139+
// Pretend that TEST_SERVICE has connected and wants to call the CLI APIs
140+
when(mockContext.getAuthenticationData()).thenReturn(() -> TEST_SERVICE);
141+
GetComponentDetailsRequest request = new GetComponentDetailsRequest();
142+
request.setComponentName("any");
143+
when(authorizationHandler.isAuthorized(eq(CLI_SERVICE), any())).thenReturn(true);
144+
when(kernel.locate("any")).thenThrow(ServiceLoadException.class);
145+
assertThrows(ResourceNotFoundError.class,
146+
() -> cliEventStreamAgent.getGetComponentDetailsHandler(mockContext).handleRequest(request));
147+
}
148+
125149
@Test
126150
void test_GetComponentDetails_empty_component_name() {
127151
GetComponentDetailsRequest request = new GetComponentDetailsRequest();

‎server/src/test/java/com/aws/greengrass/cli/CLIServiceTest.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ void testStartup_default_auth() throws Exception {
130130
cliService.startup();
131131
verifyHandlersRegisteredForAllOperations();
132132
verify(authenticationHandler).registerAuthenticationTokenForExternalClient
133-
(anyString(), startsWith("greengrass-cli-user"));
133+
(anyString(), startsWith("greengrass-cli#user"));
134134

135135
Path authDir = nucleusPaths.cliIpcInfoPath();
136136
assertTrue(Files.exists(authDir));
@@ -154,9 +154,9 @@ void testStartup_group_auth(ExtensionContext context) throws Exception {
154154
when(authenticationHandler.registerAuthenticationTokenForExternalClient(anyString(), anyString()))
155155
.thenAnswer(i -> {
156156
Object clientId = i.getArgument(1);
157-
if ("greengrass-cli-group-123".equals(clientId)) {
157+
if ("greengrass-cli#group-123".equals(clientId)) {
158158
return MOCK_AUTH_TOKEN;
159-
} else if ("greengrass-cli-group-456".equals(clientId)) {
159+
} else if ("greengrass-cli#group-456".equals(clientId)) {
160160
return MOCK_AUTH_TOKEN_2;
161161
}
162162
throw new InvalidUseOfMatchersException(
@@ -197,8 +197,8 @@ void testStartup_group_auth(ExtensionContext context) throws Exception {
197197
ArgumentCaptor<String> argument = ArgumentCaptor.forClass(String.class);
198198
verify(authenticationHandler, times(2)).registerAuthenticationTokenForExternalClient
199199
(anyString(), argument.capture());
200-
assertThat(argument.getAllValues(), containsInRelativeOrder("greengrass-cli-group-123",
201-
"greengrass-cli-group-456"));
200+
assertThat(argument.getAllValues(), containsInRelativeOrder("greengrass-cli#group-123",
201+
"greengrass-cli#group-456"));
202202

203203
Path authDir = nucleusPaths.cliIpcInfoPath();
204204
assertTrue(Files.exists(authDir.resolve("group-123")));

0 commit comments

Comments
 (0)