Check user authorization in realm api requests #975
Conversation
…k permissions in scripts, simplify and refactor
packages/realm-server/main.ts
Outdated
| @@ -9,7 +9,8 @@ import { makeFastBootIndexRunner } from './fastboot'; | |||
| import { RunnerOptionsManager } from '@cardstack/runtime-common/search-index'; | |||
| import { readFileSync } from 'fs-extra'; | |||
| import { shimExternals } from './lib/externals'; | |||
| import RealmPermissions from './lib/realm-permissions'; | |||
| import { RealmPermissions as RealmPermissionsInterface } from '@cardstack/runtime-common/realm'; | |||
There was a problem hiding this comment.
| import { RealmPermissions as RealmPermissionsInterface } from '@cardstack/runtime-common/realm'; | |
| import { type RealmPermissions as RealmPermissionsInterface } from '@cardstack/runtime-common/realm'; |
packages/realm-server/main.ts
Outdated
| @@ -157,7 +156,51 @@ let realmPermissions = new RealmPermissions(); | |||
| dist, | |||
| manager.getOptions.bind(manager), | |||
| ); | |||
| let permissions = realmPermissions.permissionsForRealm(url); | |||
|
|
|||
| let userPermissions = {} as { | |||
There was a problem hiding this comment.
consider extracting a method/util to keep realm-server/main tidy
There was a problem hiding this comment.
Good idea, I added this
| let userPermissions = [ | ||
| ...(this.realmPermissions['*'] || []), | ||
| ...(this.realmPermissions[username] || []), | ||
| ]; |
packages/runtime-common/realm.ts
Outdated
| if (e instanceof AuthorizationError) { | ||
| return new Response(`Unauthorized: ${e.message}`, { status: 401 }); | ||
| } | ||
|
|
||
| if (e instanceof PermissionError) { | ||
| return new Response(`Not allowed: ${e.message}`, { status: 403 }); | ||
| } |
There was a problem hiding this comment.
I think the language typically used for this is Authentication (who are you?) and Authorization (what are you allowed to do?). Maybe those would be better names than Authorization and Permission?
There was a problem hiding this comment.
Makes sense. I initially used AuthorizationError for authentication errors because of the Authorization header but Authenticaton and Authorization do describe this better. I adjusted this
|
I've addressed all the review comments. Also, this PR is somewhat sensitive to the deployment due to ENV var requirements and keeping things work for the world readable realms so I test deployed this branch to our staging. There was a problem that the new error from this PR handling caught successfully - the permissions config (parameter store) was faulty (realm keys were names when they should be URLs). I fixed that issue and redeployed to staging and things seem to be working ok. I've also addressed this issue in production param store. |
delete-merged-branch
bot
deleted the
cs-6438-check-authorization-of-user-on-each-api-request
branch
This PR adds authentication and authorization to all our realm's endpoints. If the http method is PUT, PATCH, POST, DELETE, then the request is considered a ✍️ write request, otherwise it's a 📖 read request.
There's a new
checkPermissionfunction that will:Notes
REALM_USER_PERMISSIONS="{}"in our realm start scripts have been removed..realms.json.testandrealms.json.developmentfiles have been introduced (permissions config) for better experience when changing permissions during development or for testing. However, in our deployed environments (notdevelopmentortest),REALM_USER_PERMISSIONSis a mandatory ENV variable. This is because in deployed environments it's easier to manage an ENV variable compared to file copying approachFuture work
Currently, all our deployed realms are public readable and writable. They will need to stay this way until we add support for realms to be be able to authenticate between each other, otherwise indexing cards from other realms (e.g. linked cards) will fail if there is no permission to read.