Skip to content

Conversation

@mustard-mh
Copy link
Contributor

@mustard-mh mustard-mh commented Jan 15, 2024

Description

Related Issue(s)

Fixes EXP-1084, EXP-1085

How to test

Without OIDC, test script mentioned below should failed. Since there's no organization owned users even you have Feature Flag enabled.

  • Try test with test script
    export TEST_CREATE_TMP_TOKEN=true
    export GITPOD_HOST=hw-token-exp-1084.preview.gitpod-dev.com
    # cookie should format like: <cookie_name>=<cookie_value>
    export USER_TOKEN=<correct_cookie>
    export MEMBER_USER_TOKEN=<member_cookie>
    export MEMBER_USER_ID=<member_id>
    export CURRENT_USER_ID=<admin_id>
    export TARGET_USER_ID=<target_user_id>
    go test -run "^TestCreateTemporaryAccessToken" github.com/gitpod-io/gitpod/test/tests/smoke-test -v -count=1
Create an OIDC preview env youself
  • Open this PR with Gitpod, create another branch based on current one
  • Exec command
export GITPOD_WITH_DEDICATED_EMU=true
leeway run dev:preview
previewctl admin credentials create
kubectl rollout restart deploy/server

# access link print via previewctl
  • Setup our SSO

Documentation

Preview status

gitpod:summary

Build Options

Build
  • /werft with-werft
    Run the build with werft instead of GHA
  • leeway-no-cache
  • /werft no-test
    Run Leeway with --dont-test
Publish
  • /werft publish-to-npm
  • /werft publish-to-jb-marketplace
Installer
  • analytics=segment
  • with-dedicated-emulation
  • workspace-feature-flags
    Add desired feature flags to the end of the line above, space separated
Preview Environment / Integration Tests
  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-large-vm
  • /werft with-gce-vm
    If enabled this will create the environment on GCE infra
  • /werft preemptible
    Saves cost. Untick this only if you're really sure you need a non-preemtible machine.
  • with-integration-tests=all
    Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh. If enabled, with-preview and with-large-vm will be enabled.
  • with-monitoring

/hold

@roboquat roboquat added size/XXL and removed size/XL labels Jan 18, 2024
@mustard-mh mustard-mh changed the title WIP [papi] add api to allow organization owner to create a temporary token [papi] add api to allow organization owner to create a temporary token Jan 19, 2024
@mustard-mh mustard-mh marked this pull request as ready for review January 19, 2024 06:27
@mustard-mh mustard-mh requested review from a team as code owners January 19, 2024 06:27
export class AdminUserAddAdminRole1705591447620 implements MigrationInterface {
public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(
`UPDATE d_b_user SET rolesOrPermissions = '["admin"]' WHERE id = '${BUILTIN_INSTLLATION_ADMIN_USER_ID}'`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need to add this role back? It looks like FGA doesn't care given this code in authorizer.ts:

export function createInitializingAuthorizer(spiceDbAuthorizer: SpiceDBAuthorizer): Authorizer {
    const target = new Authorizer(spiceDbAuthorizer);
    const initialized = (async () => {
        await target.addInstallationAdminRole(BUILTIN_INSTLLATION_ADMIN_USER_ID);
        await target.addUser(BUILTIN_INSTLLATION_ADMIN_USER_ID);
    })().catch((err) => log.error("Failed to initialize authorizer", err));
    return new Proxy(target, {
        get(target, propKey, receiver) {
            const originalMethod = target[propKey as keyof typeof target];

            if (typeof originalMethod === "function") {
                return async function (...args: any[]) {
                    await initialized;
                    return (originalMethod as any).apply(target, args);
                };
            } else {
                return originalMethod;
            }
        },
    });
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

installation#admin will be removed by regular job because it doesn't have role: admin in database

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, ok that gets executed when ever we bump the version. 👍

@mustard-mh
Copy link
Contributor Author

Feedback addressed, smoke tests pass after deployment

image

@roboquat roboquat merged commit 230c190 into main Jan 19, 2024
@roboquat roboquat deleted the hw/token-EXP-1084 branch January 19, 2024 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants