Skip to content

Commit

Permalink
[DMS-451] DMS client should use separate role from configuration serv…
Browse files Browse the repository at this point in the history
…ice (#367)

* DMS client should use separate role from configuration service

* #DMS-445 Change the Config Service scope to match Admin API

* Add audience to client scope
  • Loading branch information
simpat-adam authored Dec 10, 2024
1 parent 370d033 commit e988ef3
Show file tree
Hide file tree
Showing 24 changed files with 127 additions and 98 deletions.
7 changes: 3 additions & 4 deletions eng/docker-compose/.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,7 @@ OPENSEARCH_URL=http://dms-search:9200

# Authorization parameters
IDENTITY_ENFORCE_AUTHORIZATION=true
# Config Service is currently assigning DMS clients the same role as Config Service clients, not using "dms-client" as intended
# IDENTITY_SERVICE_ROLE=dms-client
IDENTITY_SERVICE_ROLE=config-service-app
IDENTITY_CLIENT_ROLE=dms-client
IDENTITY_AUTHORITY=http://dms-keycloak:8080/realms/edfi
IDENTITY_AUDIENCE=account
IDENTITY_REQUIRE_HTTPS_METADATA=false
Expand All @@ -94,6 +92,7 @@ DMS_CONFIG_DATASTORE=postgresql
DMS_CONFIG_DATABASE_CONNECTION_STRING=host=dms-postgresql;port=5432;username=postgres;password=${POSTGRES_PASSWORD};database=${POSTGRES_DB_NAME};
DMS_CONFIG_IDENTITY_ALLOW_REGISTRATION=true
DMS_CONFIG_IDENTITY_SERVICE_ROLE=config-service-app
DMS_CONFIG_IDENTITY_CLIENT_ROLE=dms-client
DMS_CONFIG_IDENTITY_AUTHORITY=${IDENTITY_AUTHORITY}
DMS_CONFIG_IDENTITY_AUDIENCE=${IDENTITY_AUDIENCE}
KEYCLOAK_URL=http://dms-keycloak:8080
Expand All @@ -102,6 +101,6 @@ DMS_CONFIG_IDENTITY_CLIENT_ID=DmsConfigurationService
DMS_CONFIG_IDENTITY_CLIENT_SECRET=s3creT@09
DMS_CONFIG_IDENTITY_REQUIRE_HTTPS=false
DMS_CONFIG_IDENTITY_ROLE_CLAIM_TYPE=${IDENTITY_ROLE_CLAIM_TYPE}
DMS_CONFIG_IDENTITY_SCOPE=scp:edfi_dms_configuration_service/full_access
DMS_CONFIG_IDENTITY_SCOPE=edfi_admin_api/full_access
DMS_CONFIG_LOG_LEVEL=Information
DMS_CONFIG_DEPLOY_DATABASE=true
6 changes: 4 additions & 2 deletions eng/docker-compose/KEYCLOAK-SETUP.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ Keycloak locally using docker-compose.
7. Update DMS Configuration Service IdentitySettings section on appsettings.json:
```js
ServiceRole: "config-service-app" (Service realm role created earlier)
ConfigServiceRole: "config-service-app" (Service realm role created earlier)
ClientRole: "dms-client",
Authority: "http://your-keycloak-url:port/realms/<your-realm>"
IdentityServer: "http://your-keycloak-url:port"
Realm: "edfi"(your realm)
Expand Down Expand Up @@ -313,7 +314,8 @@ Please refer "Creating a Configuration Service Client" section above

```js
"EnforceAuthorization": true,
"ServiceRole": "dms-client",
"ConfigServiceRole": "config-service-app",
"ClientRole": "dms-client",
"Authority": "http://your-keycloak-url:port/realms/<your-realm>",
"Audience": "account",
"RequireHttpsMetadata": false,
Expand Down
3 changes: 2 additions & 1 deletion eng/docker-compose/local-config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,14 @@ services:
DMS_CONFIG_DATABASE_CONNECTION_STRING: ${DMS_CONFIG_DATABASE_CONNECTION_STRING:-host=localhost;port=5432;username=postgres;database=edfi_configurationservice;}
DMS_CONFIG_IDENTITY_ALLOW_REGISTRATION: ${DMS_CONFIG_IDENTITY_ALLOW_REGISTRATION:-false}
DMS_CONFIG_IDENTITY_SERVICE_ROLE: ${DMS_CONFIG_IDENTITY_SERVICE_ROLE:-config-service-app}
DMS_CONFIG_IDENTITY_CLIENT_ROLE: ${DMS_CONFIG_IDENTITY_CLIENT_ROLE:-dms-client}
DMS_CONFIG_IDENTITY_AUTHORITY: ${DMS_CONFIG_IDENTITY_AUTHORITY:-http://localhost:8045/realms/edfi}
DMS_CONFIG_IDENTITY_AUDIENCE: ${DMS_CONFIG_IDENTITY_AUDIENCE:-account}
DMS_CONFIG_IDENTITY_CLIENT_ID: ${DMS_CONFIG_IDENTITY_CLIENT_ID:-DmsConfigurationService}
DMS_CONFIG_IDENTITY_CLIENT_SECRET: ${DMS_CONFIG_IDENTITY_CLIENT_SECRET}
DMS_CONFIG_IDENTITY_REQUIRE_HTTPS: ${DMS_CONFIG_IDENTITY_REQUIRE_HTTPS:-false}
DMS_CONFIG_IDENTITY_ROLE_CLAIM_TYPE: ${DMS_CONFIG_IDENTITY_ROLE_CLAIM_TYPE:-http://schemas\\.microsoft\\.com/ws/2008/06/identity/claims/role}
DMS_CONFIG_IDENTITY_SCOPE: ${DMS_CONFIG_IDENTITY_SCOPE:-scp:edfi_dms_configuration_service/full_access}
DMS_CONFIG_IDENTITY_SCOPE: ${DMS_CONFIG_IDENTITY_SCOPE:-edfi_admin_api/full_access}
DMS_CONFIG_LOG_LEVEL: ${DMS_CONFIG_LOG_LEVEL:-Information}
DMS_CONFIG_DEPLOY_DATABASE: ${DMS_CONFIG_DEPLOY_DATABASE:-true}
DMS_CONFIG_IDENTITY_PROVIDER: ${DMS_CONFIG_IDENTITY_PROVIDER:-keycloak}
Expand Down
2 changes: 1 addition & 1 deletion eng/docker-compose/local-dms.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ services:
BREAK_DURATION_SECONDS: ${BREAK_DURATION_SECONDS:-30}
OPENSEARCH_URL: ${OPENSEARCH_URL:-http://dms-search:9200}
IDENTITY_ENFORCE_AUTHORIZATION: ${IDENTITY_ENFORCE_AUTHORIZATION:-false}
IDENTITY_SERVICE_ROLE: ${IDENTITY_SERVICE_ROLE:-dms-client}
IDENTITY_CLIENT_ROLE: ${IDENTITY_CLIENT_ROLE:-dms-client}
IDENTITY_AUTHORITY: ${IDENTITY_AUTHORITY:-http://dms-keycloak:8080/realms/dms}
IDENTITY_AUDIENCE: ${IDENTITY_AUDIENCE:-account}
IDENTITY_REQUIRE_HTTPS_METADATA: ${IDENTITY_REQUIRE_HTTPS_METADATA:-false}
Expand Down
20 changes: 15 additions & 5 deletions eng/docker-compose/setup-keycloak.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,19 @@ function Create_ClientScope([string] $scopeName) {
else {
# Create the client scope
$clientScopePayload = @{
name = $scopeName
protocol = "openid-connect"
} | ConvertTo-Json
name = $scopeName
protocol = "openid-connect"
protocolMappers = @(@{
name = $scopeName
protocol = "openid-connect"
protocolMapper = "oidc-audience-resolve-mapper"
consentRequired = "false"
config = @{
"introspection.token.claim" = "true"
"access.token.claim" = "true"
}
})
} | ConvertTo-Json -Depth 3

Invoke-RestMethod -Uri "$KeycloakServer/admin/realms/$Realm/client-scopes" `
-Method Post `
Expand Down Expand Up @@ -269,7 +279,7 @@ function Assign_RealmRole([object] $role, [string] $ClientId) {
-Body $rolesArray `
-ContentType "application/json"

Write-Output "Role '$DmsClientRole' assigned as a service account role to client '$NewClientName'."
Write-Output "Role '$role' assigned as a service account role to clientId '$ClientId'."
}

function Assign_Realm_Admin_Role([object] $role, [string] $ClientId) {
Expand All @@ -292,7 +302,7 @@ function Assign_Realm_Admin_Role([object] $role, [string] $ClientId) {
-Body $rolesArray `
-ContentType "application/json"

Write-Output "Role 'realm-admin' assigned as a service account role to client '$NewClientName'."
Write-Output "Role 'realm-admin' assigned as a service account role to clientId '$ClientId'."
}

function Add_Role_To_Token([string] $ClientId) {
Expand Down
3 changes: 1 addition & 2 deletions eng/docker-compose/start-local-dms.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ $files = @(
"local-dms.yml"
)

if($SearchEngine -eq "ElasticSearch")
{
if ($SearchEngine -eq "ElasticSearch") {
$files += @("-f", "kafka-elasticsearch.yml")
if ($EnableSearchEngineUI) {
$files += @("-f", "kafka-elasticsearch-ui.yml")
Expand Down
3 changes: 2 additions & 1 deletion src/config/appsettings.template.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
},
"IdentitySettings": {
"AllowRegistration": "${DMS_CONFIG_IDENTITY_ALLOW_REGISTRATION}",
"ServiceRole": "${DMS_CONFIG_IDENTITY_SERVICE_ROLE}",
"ConfigServiceRole": "${DMS_CONFIG_IDENTITY_SERVICE_ROLE}",
"ClientRole": "${DMS_CONFIG_IDENTITY_CLIENT_ROLE}",
"Authority": "${DMS_CONFIG_IDENTITY_AUTHORITY}",
"Audience": "${DMS_CONFIG_IDENTITY_AUDIENCE}",
"ClientId": "${DMS_CONFIG_IDENTITY_CLIENT_ID}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,51 +14,49 @@

namespace EdFi.DmsConfigurationService.Backend.Keycloak;

public class KeycloakClientRepository(KeycloakContext keycloakContext, ILogger<KeycloakClientRepository> logger)
: IClientRepository
public class KeycloakClientRepository(
KeycloakContext keycloakContext,
ILogger<KeycloakClientRepository> logger
) : IClientRepository
{
private readonly KeycloakClient _keycloakClient =
new(
keycloakContext.Url,
keycloakContext.ClientSecret,
new KeycloakOptions(adminClientId: keycloakContext.ClientId)
);
private readonly KeycloakClient _keycloakClient = new(
keycloakContext.Url,
keycloakContext.ClientSecret,
new KeycloakOptions(adminClientId: keycloakContext.ClientId)
);
private readonly string _realm = keycloakContext.Realm;

public async Task<ClientCreateResult> CreateClientAsync(
string clientId,
string clientSecret,
string role,
string displayName
)
{
try
{
Client client =
new()
{
ClientId = clientId,
Enabled = true,
Secret = clientSecret,
Name = displayName,
ServiceAccountsEnabled = true,
DefaultClientScopes = [keycloakContext.Scope],
ProtocolMappers = ConfigServiceProtocolMapper(),
};

// Read service role from the realm
Client client = new()
{
ClientId = clientId,
Enabled = true,
Secret = clientSecret,
Name = displayName,
ServiceAccountsEnabled = true,
DefaultClientScopes = [keycloakContext.Scope],
ProtocolMappers = ConfigServiceProtocolMapper(),
};

// Read role from the realm
var realmRoles = await _keycloakClient.GetRolesAsync(_realm);
Role? clientRole = realmRoles.FirstOrDefault(x =>
x.Name.Equals(keycloakContext.ServiceRole, StringComparison.InvariantCultureIgnoreCase)
x.Name.Equals(role, StringComparison.InvariantCultureIgnoreCase)
);

if (clientRole is null)
{
await _keycloakClient.CreateRoleAsync(
_realm,
new Role() { Name = keycloakContext.ServiceRole }
);
await _keycloakClient.CreateRoleAsync(_realm, new Role() { Name = role });

clientRole = await _keycloakClient.GetRoleByNameAsync(_realm, keycloakContext.ServiceRole);
clientRole = await _keycloakClient.GetRoleByNameAsync(_realm, role);
}

var clientScopes = await _keycloakClient.GetClientScopesAsync(_realm);
Expand Down Expand Up @@ -117,9 +115,7 @@ await _keycloakClient.CreateClientScopeAsync(
}
else
{
return new ClientCreateResult.FailureUnknown(
$"Role {keycloakContext.ServiceRole} not found."
);
return new ClientCreateResult.FailureUnknown($"Role {role} not found.");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,5 @@ public record KeycloakContext(
string ClientId,
string ClientSecret,
string RoleClaimType,
string ServiceRole,
string Scope
);
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
namespace EdFi.DmsConfigurationService.Backend.Keycloak;

/// <summary>
/// Service extensions to register keycloak access points
/// Service extensions to register keycloak access points
/// </summary>
public static class KeycloakServiceExtensions
{
Expand All @@ -20,16 +20,15 @@ public static IServiceCollection AddKeycloakServices(
string ClientId,
string ClientSecret,
string RoleClaimType,
string ServiceRole,
string Scope)
string Scope
)
{
services.AddScoped(x => new KeycloakContext(
Url,
Realm,
ClientId,
ClientSecret,
RoleClaimType,
ServiceRole,
Scope
));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public interface IClientRepository
public Task<ClientCreateResult> CreateClientAsync(
string clientId,
string clientSecret,
string role,
string displayName
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ public void Setup()
A.CallTo(
() =>
_clientRepository.CreateClientAsync(
A<string>.Ignored,
A<string>.Ignored,
A<string>.Ignored,
A<string>.Ignored
Expand Down Expand Up @@ -309,6 +310,7 @@ public void SetUp()
A.CallTo(
() =>
_clientRepository.CreateClientAsync(
A<string>.Ignored,
A<string>.Ignored,
A<string>.Ignored,
A<string>.Ignored
Expand Down Expand Up @@ -488,6 +490,7 @@ public void SetUp()
A.CallTo(
() =>
_clientRepository.CreateClientAsync(
A<string>.Ignored,
A<string>.Ignored,
A<string>.Ignored,
A<string>.Ignored
Expand Down Expand Up @@ -615,6 +618,7 @@ public void SetUp()
A.CallTo(
() =>
_clientRepository.CreateClientAsync(
A<string>.Ignored,
A<string>.Ignored,
A<string>.Ignored,
A<string>.Ignored
Expand Down Expand Up @@ -694,7 +698,6 @@ public async Task Should_return_bad_request_due_to_duplicate_claim_set_name_on_u
A.CallTo(() => _applicationRepository.UpdateApplication(A<ApplicationUpdateCommand>.Ignored))
.Returns(new ApplicationUpdateResult.FailureDuplicateClaimSetName());


//Act
var updateResponse = await client.PutAsync(
"/v2/applications/1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public void Setup()
A.CallTo(
() =>
_clientRepository.CreateClientAsync(
A<string>.Ignored,
A<string>.Ignored,
A<string>.Ignored,
A<string>.Ignored
Expand Down Expand Up @@ -445,7 +446,7 @@ public async Task Given_valid_client_credentials()
new KeyValuePair<string, string>("client_id", "CSClient1"),
new KeyValuePair<string, string>("client_secret", "test123@Puiu"),
new KeyValuePair<string, string>("grant_type", "client_credentials"),
new KeyValuePair<string, string>("scope", "scp:edfi_dms_configuration_service/full_access"),
new KeyValuePair<string, string>("scope", "edfi_admin_api/full_access"),
}
);
var response = await client.PostAsync("/connect/token", requestContent);
Expand Down Expand Up @@ -532,7 +533,7 @@ public async Task When_error_from_backend()
new KeyValuePair<string, string>("client_id", "CSClient1"),
new KeyValuePair<string, string>("client_secret", "test123@Puiu"),
new KeyValuePair<string, string>("grant_type", "client_credentials"),
new KeyValuePair<string, string>("scope", "scp:edfi_dms_configuration_service/full_access"),
new KeyValuePair<string, string>("scope", "edfi_admin_api/full_access"),
}
);
var response = await client.PostAsync("/connect/token", requestContent);
Expand Down Expand Up @@ -581,7 +582,7 @@ public async Task When_provider_is_unreacheable()
new KeyValuePair<string, string>("client_id", "CSClient1"),
new KeyValuePair<string, string>("client_secret", "test123@Puiu"),
new KeyValuePair<string, string>("grant_type", "client_credentials"),
new KeyValuePair<string, string>("scope", "scp:edfi_dms_configuration_service/full_access"),
new KeyValuePair<string, string>("scope", "edfi_admin_api/full_access"),
}
);
var response = await client.PostAsync("/connect/token", requestContent);
Expand All @@ -598,7 +599,7 @@ public async Task When_provider_is_unreacheable()
"title": "Bad Gateway",
"status": 502,
"correlationId": "{correlationId}",
"validationErrors": {},
"validationErrors": {},
"errors": []
}
""".Replace("{correlationId}", actualResponse!["correlationId"]!.GetValue<string>())
Expand Down Expand Up @@ -644,7 +645,7 @@ public async Task When_provider_has_invalid_realm()
new KeyValuePair<string, string>("client_id", "CSClient1"),
new KeyValuePair<string, string>("client_secret", "test123@Puiu"),
new KeyValuePair<string, string>("grant_type", "client_credentials"),
new KeyValuePair<string, string>("scope", "scp:edfi_dms_configuration_service/full_access"),
new KeyValuePair<string, string>("scope", "edfi_admin_api/full_access"),
}
);
var response = await client.PostAsync("/connect/token", requestContent);
Expand Down Expand Up @@ -693,7 +694,7 @@ public async Task When_provider_has_not_realm_admin_role()
new KeyValuePair<string, string>("client_id", "CSClient1"),
new KeyValuePair<string, string>("client_secret", "test123@Puiu"),
new KeyValuePair<string, string>("grant_type", "client_credentials"),
new KeyValuePair<string, string>("scope", "scp:edfi_dms_configuration_service/full_access"),
new KeyValuePair<string, string>("scope", "edfi_admin_api/full_access"),
}
);
var response = await client.PostAsync("/connect/token", requestContent);
Expand Down Expand Up @@ -736,7 +737,7 @@ public async Task When_provider_has_bad_credetials()
new KeyValuePair<string, string>("client_id", "CSClient1"),
new KeyValuePair<string, string>("client_secret", "test123@Puiu"),
new KeyValuePair<string, string>("grant_type", "client_credentials"),
new KeyValuePair<string, string>("scope", "scp:edfi_dms_configuration_service/full_access"),
new KeyValuePair<string, string>("scope", "edfi_admin_api/full_access"),
}
);
var response = await client.PostAsync("/connect/token", requestContent);
Expand Down
Loading

0 comments on commit e988ef3

Please sign in to comment.