Tenant Issuer Config Related Fixes #737

shaangill025 · 2023-07-27T16:31:16Z

resolve Configuration to create Tenants with default permission values #607
resolve Block endorser connection and public DID tenant endpoints if Innkeeper approval is not set #608
resolve Can't clear the connect_to_endorser and create_public_did fields in config #667

Signed-off-by: Shaanjot Gill <[email protected]>

github-actions · 2023-07-27T21:31:45Z

Deployment	Resource	Location
Traction
	Tenant UI	https://pr-737-tenant-ui-dev.apps.silver.devops.gov.bc.ca
	Innkeeper UI	https://pr-737-tenant-ui-dev.apps.silver.devops.gov.bc.ca/innkeeper
Aca-Py
	Admin Swagger	https://pr-737-traction-acapy-admin-dev.apps.silver.devops.gov.bc.ca/api/doc
	Tenant Proxy Swagger	https://pr-737-traction-tenant-proxy-dev.apps.silver.devops.gov.bc.ca/api/doc

Deployment URLs ready for review.

loneil

It's possible I'm really misunderstanding what the self_issuer_permission flag is to do here? Or there's some fixes needed.

I think the intention is if self_issuer_permission is true then a tenant that gets created can then connect to the endorser and register a DID without Innkeeper intervention right?

Trying locally on Docker, if I flip self_issuer_permission to true then create a new Tenant, I'm still seeing the blockage on the issuance switches (see below)
This might just be a minor Tenant UI code bug from a change in this PR, see attached comments in code.

I tried out switching up the conditional there in Vue locally, and the switches appeared as I would expect (again if I'm understanding the permission flag), however when trying to connect to the endorser it's still blocking the user:

If I'm a Tenant with self_issuer_permission true in my config I should be able to connect to the endorser without Innkeeper intervention right?

Then also, on the PR here (https://pr-737-tenant-ui-dev.apps.silver.devops.gov.bc.ca/innkeeper), where self_issuer_permission is false. I'm now unable to set the endorser and DID permissions as the Innkeeper.
It's expecting the self_issuer_permission field to be PUT back with the other config. Maybe this is just a matter of the Tenant UI sending back the current config value for that back in the PUT? If that's the intended flow? I know some other settings PUT commands around ACA-Py don't enforce including every field in the body and then it doesn't change it if it's not supplied (not sure if that's a consistent pattern or not)
I guess it could be nice in the event that self_issuer_permission is ON, that the Innkeeper could revoke that 🤷 So could have a switch for it as well, but that could be a future consideration. Need to not get the 422s for now.

loneil · 2023-07-29T18:52:17Z

services/tenant-ui/frontend/src/components/profile/issuance/Endorser.vue

+  if (
+    tenantConfig.value?.connect_to_endorser?.length > 0 ||
+    tenantConfig.value?.self_issuer_permission
+  ) {


Maybe something like

if(tenantConfig.value?.self_issuer_permission) { // Don't need to check, if the tenant is a "self issuer" return true; } if (tenantConfig.value?.connect_to_endorser?.length ) { . . .

loneil · 2023-07-29T18:52:36Z

services/tenant-ui/frontend/src/components/profile/issuance/PublicDid.vue

+  if (
+    tenantConfig.value?.create_public_did?.length > 0 ||
+    tenantConfig.value?.self_issuer_permission
+  ) {


Maybe something like

if(tenantConfig.value?.self_issuer_permission) { // Don't need to check, if the tenant is a "self issuer" return true; } if (tenantConfig.value?.create_public_did?.length ) { . . .

loneil · 2023-07-30T01:30:37Z

Aside from comments above also a question probably for @esune for tracking future. Your idea for #607 was the endorser and did registration settings to be pulled as configured defaults (see 607 AC) for the endorser(s) and ledger(s).
I'm definitely content doing this self_issuer_permission boolean instead, but not sure if you want to keep #607 around for other future plans rather than close with this implementation? Maybe needs revisiting with future multi-ledger stuff (or just have this single self_issuer_permission be the permanent solution)

loneil · 2023-08-01T04:03:33Z

Unblocking PR review as I am out of service until Aug5 in case someone else needs to look at resolution of issues above and merge.

esune · 2023-08-01T16:29:46Z

Talked with @shaangill025 this morning: the self_issuer_permission flag should only be used to drive the logic at the innkeeper level, without UI changes. If the self_issuer_permission is set to true, on tenant approval (manual or automatic) the settings enabling the tenant to connect to the endorser and publish their DID would be copied to the tenat's configuration, similarly to what happens when explicitly turning them on via UI/API.

Signed-off-by: Shaanjot Gill <[email protected]>

…into tenant_config_fix

…config_fix Signed-off-by: Shaanjot Gill <[email protected]>

Signed-off-by: Shaanjot Gill <[email protected]>

esune · 2023-08-03T15:42:05Z

plugins/traction_innkeeper/traction_innkeeper/v1_0/innkeeper/utils.py

+        description="True if tenant can make itself issuer, false if only innkeeper can",
+        default=False,


Might be better described as True if tenants are automatically allowed to be issuers, False if innkeeper approval is required". Maybe the flag needs to be renamed as well to something more consistent to the behaviour, like auto_issuer_permission`?

Changed to auto_issuer to be consistent with other flags in reservation config like auto_approve.

Signed-off-by: Shaanjot Gill <[email protected]>

loneil · 2023-08-08T16:57:23Z

@shaangill025 unable to get a token as innkeeper tenant and seeing the following error in acapy logs.

return await handler(request)
File "/home/aries/.venv/lib/python3.9/site-packages/aries_cloudagent/admin/server.py", line 330, in check_token
return await handler(request)
File "/home/aries/.venv/lib/python3.9/site-packages/aries_cloudagent/admin/server.py", line 379, in check_multitenant_authorization
return await handler(request)
File "/home/aries/.venv/lib/python3.9/site-packages/aries_cloudagent/admin/server.py", line 442, in setup_context
return await task
File "/usr/local/lib/python3.9/asyncio/futures.py", line 284, in __await__
yield self # This tells Task to wait for completion.
File "/usr/local/lib/python3.9/asyncio/tasks.py", line 328, in __wakeup
future.result()
File "/usr/local/lib/python3.9/asyncio/futures.py", line 201, in result
raise self._exception
File "/usr/local/lib/python3.9/asyncio/tasks.py", line 256, in __step
result = coro.send(None)
File "/home/aries/traction_plugins/traction_innkeeper/v1_0/innkeeper/routes.py", line 88, in wrapper
raise err
File "/home/aries/traction_plugins/traction_innkeeper/v1_0/innkeeper/routes.py", line 72, in wrapper
ret = await func(request)
File "/home/aries/traction_plugins/traction_innkeeper/v1_0/innkeeper/routes.py", line 422, in tenant_create_token
tenant_record = await TenantRecord.retrieve_by_id(session, tenant_id)
File "/home/aries/.venv/lib/python3.9/site-packages/aries_cloudagent/messaging/models/base_record.py", line 243, in retrieve_by_id
return cls.from_storage(record_id, vals)
File "/home/aries/.venv/lib/python3.9/site-packages/aries_cloudagent/messaging/models/base_record.py", line 123, in from_storage
return cls(**params)
File "/home/aries/traction_plugins/traction_innkeeper/v1_0/innkeeper/models.py", line 266, in __init__
super().__init__(
TypeError: __init__() got an unexpected keyword argument 'self_issuer_permission'

shaangill025 · 2023-08-08T17:01:01Z

@loneil All references to self_issuer_permission has been changed to auto_issuer.

shaangill025 · 2023-08-08T17:24:48Z

Reopened as #748

shaangill025 added 3 commits July 26, 2023 12:24

fix #608, #607

2a965f1

Signed-off-by: Shaanjot Gill <[email protected]>

#607 inital impl

935891f

Signed-off-by: Shaanjot Gill <[email protected]>

fix missing config values

3a206ea

Signed-off-by: Shaanjot Gill <[email protected]>

shaangill025 marked this pull request as ready for review July 27, 2023 21:22

shaangill025 temporarily deployed to development July 27, 2023 21:28 — with GitHub Actions Inactive

Merge branch 'main' into tenant_config_fix

4de32ac

esune requested a review from loneil July 27, 2023 23:57

shaangill025 temporarily deployed to development July 27, 2023 23:59 — with GitHub Actions Inactive

loneil requested changes Jul 29, 2023

View reviewed changes

loneil mentioned this pull request Jul 29, 2023

How do I change the Innkeeper password, key using docker compose? #739

Closed

loneil self-requested a review August 1, 2023 04:02

shaangill025 added 3 commits August 2, 2023 08:48

changes based on feedback

9803631

Signed-off-by: Shaanjot Gill <[email protected]>

Merge branch 'tenant_config_fix' of https://github.com/bcgov/traction …

a55d410

…into tenant_config_fix

Merge branch 'main' of https://github.com/bcgov/traction into tenant_…

8458cea

…config_fix Signed-off-by: Shaanjot Gill <[email protected]>

shaangill025 had a problem deploying to development August 2, 2023 15:58 — with GitHub Actions Failure

editConfigForm bugfix

2d6a038

Signed-off-by: Shaanjot Gill <[email protected]>

shaangill025 had a problem deploying to development August 2, 2023 16:10 — with GitHub Actions Failure

shaangill025 had a problem deploying to development August 2, 2023 18:11 — with GitHub Actions Failure

shaangill025 had a problem deploying to development August 2, 2023 18:43 — with GitHub Actions Failure

shaangill025 had a problem deploying to development August 2, 2023 19:42 — with GitHub Actions Failure

syntax fix after merge

a74eadd

Signed-off-by: Shaanjot Gill <[email protected]>

shaangill025 had a problem deploying to development August 2, 2023 20:23 — with GitHub Actions Failure

shaangill025 had a problem deploying to development August 2, 2023 20:37 — with GitHub Actions Failure

shaangill025 had a problem deploying to development August 2, 2023 22:05 — with GitHub Actions Failure

shaangill025 had a problem deploying to development August 2, 2023 22:14 — with GitHub Actions Failure

shaangill025 had a problem deploying to development August 2, 2023 22:16 — with GitHub Actions Failure

shaangill025 temporarily deployed to development August 2, 2023 22:25 — with GitHub Actions Inactive

esune reviewed Aug 3, 2023

View reviewed changes

self_issuer flag name change + black fmt changes

12acd7b

Signed-off-by: Shaanjot Gill <[email protected]>

shaangill025 temporarily deployed to development August 3, 2023 17:42 — with GitHub Actions Inactive

shaangill025 closed this Aug 8, 2023

shaangill025 temporarily deployed to development August 8, 2023 17:20 — with GitHub Actions Inactive

shaangill025 mentioned this pull request Aug 8, 2023

Tenant Issuer Config Fixes #748

Merged

shaangill025 temporarily deployed to development August 8, 2023 17:30 — with GitHub Actions Inactive

esune temporarily deployed to development August 8, 2023 22:05 — with GitHub Actions Inactive

esune deleted the tenant_config_fix branch August 8, 2023 22:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tenant Issuer Config Related Fixes #737

Tenant Issuer Config Related Fixes #737

shaangill025 commented Jul 27, 2023 •

edited

Loading

github-actions bot commented Jul 27, 2023

loneil left a comment •

edited

Loading

loneil Jul 29, 2023

loneil Jul 29, 2023

loneil commented Jul 30, 2023

loneil commented Aug 1, 2023

esune commented Aug 1, 2023

esune Aug 3, 2023

shaangill025 Aug 3, 2023

loneil commented Aug 8, 2023

shaangill025 commented Aug 8, 2023

shaangill025 commented Aug 8, 2023

		description="True if tenant can make itself issuer, false if only innkeeper can",
		default=False,

Tenant Issuer Config Related Fixes #737

Tenant Issuer Config Related Fixes #737

Conversation

shaangill025 commented Jul 27, 2023 • edited Loading

github-actions bot commented Jul 27, 2023

loneil left a comment • edited Loading

Choose a reason for hiding this comment

loneil Jul 29, 2023

Choose a reason for hiding this comment

loneil Jul 29, 2023

Choose a reason for hiding this comment

loneil commented Jul 30, 2023

loneil commented Aug 1, 2023

esune commented Aug 1, 2023

esune Aug 3, 2023

Choose a reason for hiding this comment

shaangill025 Aug 3, 2023

Choose a reason for hiding this comment

loneil commented Aug 8, 2023

shaangill025 commented Aug 8, 2023

shaangill025 commented Aug 8, 2023

shaangill025 commented Jul 27, 2023 •

edited

Loading

loneil left a comment •

edited

Loading