Skip to content

Commit

Permalink
Merge branch 'feature/refactor-login-process-883' into feature/automa…
Browse files Browse the repository at this point in the history
…ted-authz-testing-1461
  • Loading branch information
oharsta committed Oct 23, 2024
2 parents 3f993b7 + 4340a8a commit 1a43464
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 30 deletions.
5 changes: 4 additions & 1 deletion client/src/api/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,10 @@ export function resetOidcClientSecret(service) {
}

export function resetScimBearerToken(service, scim_bearer_token) {
const body = {scim_bearer_token: scim_bearer_token}
const body = {
scim_bearer_token: scim_bearer_token,
scim_url: service.scim_url
};
return postPutJson(`/api/services/reset_scim_bearer_token/${service.id}`, body, "put");
}

Expand Down
2 changes: 2 additions & 0 deletions client/src/locale/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -1060,7 +1060,9 @@ const en = {
},
scim_token: {
preTitle: "For security reasons, the current SCIM token can not be displayed. ",
preTitleNoToken: "The SCIM push requires a bearer security token. ",
title: "Change the SCIM token.",
titleNoToken: "Add a SCIM token.",
confirmation: "Please enter the SCIM token for {{name}}. It will be stored encrypted.",
success: "SCIM token has been updated.",
},
Expand Down
2 changes: 2 additions & 0 deletions client/src/locale/nl.js
Original file line number Diff line number Diff line change
Expand Up @@ -1060,7 +1060,9 @@ const nl = {
},
scim_token: {
preTitle: "Om veiligheidsredenen kan het huidige SCIM token niet worden weergegeven. ",
preTitleNoToken: "Een SCIM push vereist een bearer security token. ",
title: "Verander het SCIM token.",
titleNoToken: "Voeg een SCIM token toe.",
confirmation: "Voer het SCIM token in voor {{name}}. Het zal encrypted worden opgeslagen.",
success: "SCIM token is veranderd.",
},
Expand Down
45 changes: 26 additions & 19 deletions client/src/pages/ServiceOverview.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -371,17 +371,19 @@ class ServiceOverview extends React.Component {
confirmationDialogQuestion: I18n.t("service.scim_token.confirmation", {name: service.name}),
confirmationTxt: I18n.t("confirmationDialog.confirm"),
});

} else {
const {scimBearerToken} = this.state;
resetScimBearerToken(service, scimBearerToken).then(() => {
setFlash(I18n.t("service.scim_token.success"));
this.setState({
confirmationDialogOpen: false,
scimTokenChange: false,
loading: false
});
})
resetScimBearerToken(service, scimBearerToken)
.then(() => {
setFlash(I18n.t("service.scim_token.success"));
this.setState({
confirmationDialogOpen: false,
scimTokenChange: false,
loading: false,
scimBearerToken: null,
"service": {...service, has_scim_bearer_token: true}
});
})
}
}

Expand Down Expand Up @@ -862,7 +864,8 @@ class ServiceOverview extends React.Component {
});
}

renderButtons = (isAdmin, isServiceAdmin, disabledSubmit, currentTab, showServiceAdminView, createNewServiceToken, service) => {
renderButtons = (isAdmin, isServiceAdmin, disabledSubmit, currentTab, showServiceAdminView, createNewServiceToken,
service, invalidInputs) => {
const invalidTabsMsg = this.getInvalidTabs();
return <>
{!createNewServiceToken && <div className={"actions-container"}>
Expand All @@ -875,7 +878,7 @@ class ServiceOverview extends React.Component {
{currentTab === "SCIMServer" &&
<Tooltip tip={I18n.t("service.sweep.testTooltip")} children={
<Button txt={I18n.t("service.sweep.test")}
disabled={!service.scim_enabled || (!isAdmin && !isServiceAdmin)}
disabled={!service.scim_enabled || (!isAdmin && !isServiceAdmin) || isEmpty(service.scim_url) || invalidInputs.scim_url}
onClick={() => this.doSweep(service)}/>
}/>}
<Button disabled={disabledSubmit || (!isAdmin && !isServiceAdmin)}
Expand Down Expand Up @@ -995,13 +998,16 @@ class ServiceOverview extends React.Component {
<Tooltip tip={I18n.t("scim.scimBearerTokenTooltip")}/>
</label>
<div className="scim-token-link">
<span>{I18n.t("service.scim_token.preTitle")}
{(isAdmin || isServiceAdmin) && <a href="/scim"
onClick={e => {
stopEvent(e);
this.scimTokenChangeAction(true)
}}>{I18n.t("service.scim_token.title")}</a>}
</span>
<span>{I18n.t(`service.scim_token.preTitle${service.has_scim_bearer_token ? "" : "NoToken"}`)}
{(isAdmin || isServiceAdmin) &&
<a href="/scim"
onClick={e => {
stopEvent(e);
this.scimTokenChangeAction(true)
}}>
{I18n.t(`service.scim_token.title${service.has_scim_bearer_token ? "" : "NoToken"}`)}
</a>}
</span>
</div>
</div>}

Expand Down Expand Up @@ -1805,7 +1811,8 @@ class ServiceOverview extends React.Component {
<h2 className="section-separator">{I18n.t(`serviceDetails.toc.${currentTab}`)}</h2>
{this.renderCurrentTab(config, currentTab, service, alreadyExists, isAdmin, isServiceAdmin, disabledSubmit,
invalidInputs, hasAdministrators, showServiceAdminView, createNewServiceToken, initial, crmOrganisations)}
{this.renderButtons(isAdmin, isServiceAdmin, disabledSubmit, currentTab, showServiceAdminView, createNewServiceToken, service)}
{this.renderButtons(isAdmin, isServiceAdmin, disabledSubmit, currentTab, showServiceAdminView,
createNewServiceToken, service, invalidInputs)}
</div>
</div>);
}
Expand Down
28 changes: 21 additions & 7 deletions server/api/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,8 @@ def service_by_id(service_id):
.options(selectinload(Service.service_tokens)) \
.options(selectinload(Service.service_groups))
service = query.filter(Service.id == service_id).one()
return service, 200
else:
service = query.filter(Service.id == service_id).one()
if api_call:
query = query \
.options(selectinload(Service.ip_networks))
Expand All @@ -303,7 +304,9 @@ def service_by_id(service_id):
del res["logo"]
return res, 200

return query.filter(Service.id == service_id).one(), 200
res = jsonify(service).json
res["has_scim_bearer_token"] = service.scim_bearer_token_db_value() is not None
return res, 200


@service_api.route("/all", strict_slashes=False)
Expand Down Expand Up @@ -580,7 +583,7 @@ def update_service():
for attr in [fb for fb in forbidden if fb in data]:
data[attr] = getattr(service, attr)

for attr in ["sweep_scim_last_run", "ldap_password", "scim_bearer_token", "oidc_client_secret"]:
for attr in ["sweep_scim_last_run", "ldap_password", "scim_bearer_token", "oidc_client_secret", "exported_at"]:
if attr in data:
del data[attr]

Expand All @@ -596,15 +599,24 @@ def update_service():

scim_url_changed = data.get("scim_url", None) != service.scim_url and bool(service.scim_bearer_token_db_value())
# Before we update we need to get the unencrypted bearer_token
if scim_url_changed:
scim_enabled = data.get("scim_enabled", False)
if scim_url_changed and scim_enabled:
plain_bearer_token = decrypt_scim_bearer_token(service)

if not scim_enabled:
# Clean up the Scim related attributes
data["scim_url"] = None
data["scim_bearer_token"] = None
data["sweep_scim_enabled"] = False
data["sweep_remove_orphans"] = False
data["sweep_scim_daily_rate"] = None

res = update(Service, custom_json=data, allow_child_cascades=False, allowed_child_collections=["ip_networks"])
service = res[0]

sync_external_service(current_app, service)

if scim_url_changed and service.scim_enabled:
if scim_url_changed and scim_enabled:
service.scim_bearer_token = plain_bearer_token
encrypt_scim_bearer_token(service)

Expand Down Expand Up @@ -683,7 +695,9 @@ def reset_oidc_client_secret(service_id):
def reset_scim_bearer_token(service_id):
confirm_service_admin(service_id)
service = db.session.get(Service, service_id)
# Ensure we only change the scim_bearer_token
service.scim_bearer_token = current_request.get_json()["scim_bearer_token"]
# Ensure we only change the scim_bearer_token and optional the url
data = current_request.get_json()
service.scim_bearer_token = data.get("scim_bearer_token")
service.scim_url = data.get("scim_url", service.scim_url)
encrypt_scim_bearer_token(service)
return {}, 201
10 changes: 8 additions & 2 deletions server/test/api/test_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from server.test.seed import service_mail_name, service_network_entity_id, unihard_name, \
service_network_name, service_scheduler_name, service_wiki_name, service_storage_name, \
service_cloud_name, service_storage_entity_id, service_ssh_name, unifra_name, unihard_secret, \
user_jane_name, user_roger_name, service_sram_demo_sp, umcpekela_name
user_jane_name, user_roger_name, service_sram_demo_sp, umcpekela_name, service_monitor_name


class TestService(AbstractTest):
Expand Down Expand Up @@ -63,6 +63,12 @@ def test_find_by_id_service_admin(self):
service_details = self.get(f"api/services/{service.id}", response_status_code=200, with_basic_auth=False)
self.assertEqual(unihard_name, service_details["organisation_name"])

def test_find_by_id_service_admin_has_bearer_token(self):
service = self.find_entity_by_name(Service, service_monitor_name)
self.login("urn:service_admin")
service_details = self.get(f"api/services/{service.id}", response_status_code=200, with_basic_auth=False)
self.assertTrue(service_details["has_scim_bearer_token"])

def test_find_by_id_api_call(self):
service = self.find_entity_by_name(Service, service_scheduler_name)
service = self.get(f"api/services/{service.id}")
Expand Down Expand Up @@ -696,7 +702,7 @@ def test_service_update_scim_secret_exception(self):
with self.assertLogs() as cm:
self.put(f"/api/services/reset_scim_bearer_token/{service['id']}",
{"scim_bearer_token": "somethingelse"}, response_status_code=400)
self.assertIn('"encrypt_scim_bearer_token requires scim_bearer_token and scim_url"', "\n".join(cm.output))
self.assertIn('"encrypt_scim_bearer_token requires scim_bearer_token and scim_url"', "\n".join(cm.output))

def test_access_allowed_for_crm_organisation(self):
service = self.find_entity_by_name(Service, service_cloud_name)
Expand Down
3 changes: 2 additions & 1 deletion server/test/seed.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
service_storage_entity_id = "https://storage"
service_cloud_entity_id = "https://cloud"
service_scheduler_entity_id = "uuc_scheduler_entity_id"
service_monitor_name = "LDAP/SCIM Monitor Service"

service_storage_name = "Storage"
service_wireless_name = "Wireless"
Expand Down Expand Up @@ -461,7 +462,7 @@ def seed(db, app_config, skip_seed=False):
persist_instance(db, mail, wireless, cloud, storage, wiki, network, service_ssh, uuc_scheduler,
service_empty, demo_sp, demo_rp)

service_monitor = Service(entity_id="https://ldap-monitor.example.org", name="LDAP/SCIM Monitor Service",
service_monitor = Service(entity_id="https://ldap-monitor.example.org", name=service_monitor_name,
description="Used for monitoring LDAP and SCIM. NIET AANKOMEN.",
override_access_allowed_all_connections=False, automatic_connection_allowed=True,
logo=read_image("ldap.png"),
Expand Down

0 comments on commit 1a43464

Please sign in to comment.