Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix UI when editing database roles #24660

Merged
merged 5 commits into from Jan 5, 2024

Conversation

remilapeyre
Copy link
Contributor

When using a database role the UI will try to update the database connection associated to the role. This is to make sure that the role is allowed to use this connection:

async _updateAllowedRoles(store, { role, backend, db, type = 'add' }) {
  const connection = await store.queryRecord('database/connection', { backend, id: db });
  const roles = [...connection.allowed_roles];
  const allowedRoles = type === 'add' ? addToArray([roles, role]) : removeFromArray([roles, role]);
  connection.allowed_roles = allowedRoles;
  return connection.save();
},

async createRecord(store, type, snapshot) {
  const serializer = store.serializerFor(type.modelName);
  const data = serializer.serialize(snapshot);
  const roleType = snapshot.attr('type');
  const backend = snapshot.attr('backend');
  const id = snapshot.attr('name');
  const db = snapshot.attr('database');
  try {
    await this._updateAllowedRoles(store, {
      role: id,
      backend,
      db: db[0],
    });
  } catch (e) {
    throw new Error('Could not update allowed roles for selected database. Check Vault logs for details');
  }

  return this.ajax(this.urlFor(backend, id, roleType), 'POST', { data }).then(() => {
    // ember data doesn't like 204s if it's not a DELETE
    return {
      data: assign({}, data, { id }),
    };
  });
},

This is intended to help the administrator as the role will only work if it is allowed by the database connection.

This is however an issue if the person doing the update does not have the permission to update the connection: they will not be able to use the UI to update the role even though they have the appropriate permissions to do so (using the CLI or the API will work for example).

This is often the case when the database connections are created by a centralized system but a human operator needs to create the roles.

You can try this with the following test case:

$ cat main.tf
resource "vault_auth_backend" "userpass" {
  type = "userpass"
}

resource "vault_generic_endpoint" "alice" {
  depends_on           = [vault_auth_backend.userpass]
  path                 = "auth/userpass/users/alice"
  ignore_absent_fields = true

  data_json = jsonencode({
    "policies" : ["root"],
    "password" : "alice"
  })
}

data "vault_policy_document" "db_admin" {
  rule {
    path         = "database/roles/*"
    capabilities = ["create", "read", "update", "delete", "list"]
  }
}

resource "vault_policy" "db_admin" {
  name   = "db-admin"
  policy = data.vault_policy_document.db_admin.hcl
}

resource "vault_generic_endpoint" "bob" {
  depends_on           = [vault_auth_backend.userpass]
  path                 = "auth/userpass/users/bob"
  ignore_absent_fields = true

  data_json = jsonencode({
    "policies" : [vault_policy.db_admin.name],
    "password" : "bob"
  })
}

resource "vault_mount" "db" {
  path = "database"
  type = "database"
}

resource "vault_database_secret_backend_connection" "postgres" {
  backend           = vault_mount.db.path
  name              = "postgres"
  allowed_roles     = ["*"]
  verify_connection = false

  postgresql {
    connection_url = "postgres://username:password@localhost/database"
  }
}
$ terraform apply --auto-approve

then using bob to create a role associated to the postgres connection.

This patch changes the way the UI does the update: it still tries to update the database connection but if it fails to do so because it does not have the permission it just silently skip this part and updates the role.

This also update the error message returned to the user in case of issues to include the actual errors.

Before the change

Enregistrement.de.l.ecran.2024-01-03.a.16.52.52.mov

After the change

Enregistrement.de.l.ecran.2024-01-03.a.16.55.01.mov

When using a database role the UI will try to update the database connection
associated to the role. This is to make sure that the role is allowed to
use this connection:

    async _updateAllowedRoles(store, { role, backend, db, type = 'add' }) {
      const connection = await store.queryRecord('database/connection', { backend, id: db });
      const roles = [...connection.allowed_roles];
      const allowedRoles = type === 'add' ? addToArray([roles, role]) : removeFromArray([roles, role]);
      connection.allowed_roles = allowedRoles;
      return connection.save();
    },

    async createRecord(store, type, snapshot) {
      const serializer = store.serializerFor(type.modelName);
      const data = serializer.serialize(snapshot);
      const roleType = snapshot.attr('type');
      const backend = snapshot.attr('backend');
      const id = snapshot.attr('name');
      const db = snapshot.attr('database');
      try {
        await this._updateAllowedRoles(store, {
          role: id,
          backend,
          db: db[0],
        });
      } catch (e) {
        throw new Error('Could not update allowed roles for selected database. Check Vault logs for details');
      }

      return this.ajax(this.urlFor(backend, id, roleType), 'POST', { data }).then(() => {
        // ember data doesn't like 204s if it's not a DELETE
        return {
          data: assign({}, data, { id }),
        };
      });
    },

This is intended to help the administrator as the role will only work if
it is allowed by the database connection.

This is however an issue if the person doing the update does not have
the permission to update the connection: they will not be able to use
the UI to update the role even though they have the appropriate permissions
to do so (using the CLI or the API will work for example).

This is often the case when the database connections are created by a
centralized system but a human operator needs to create the roles.

You can try this with the following test case:

    $ cat main.tf
    resource "vault_auth_backend" "userpass" {
      type = "userpass"
    }

    resource "vault_generic_endpoint" "alice" {
      depends_on           = [vault_auth_backend.userpass]
      path                 = "auth/userpass/users/alice"
      ignore_absent_fields = true

      data_json = jsonencode({
        "policies" : ["root"],
        "password" : "alice"
      })
    }

    data "vault_policy_document" "db_admin" {
      rule {
        path         = "database/roles/*"
        capabilities = ["create", "read", "update", "delete", "list"]
      }
    }

    resource "vault_policy" "db_admin" {
      name   = "db-admin"
      policy = data.vault_policy_document.db_admin.hcl
    }

    resource "vault_generic_endpoint" "bob" {
      depends_on           = [vault_auth_backend.userpass]
      path                 = "auth/userpass/users/bob"
      ignore_absent_fields = true

      data_json = jsonencode({
        "policies" : [vault_policy.db_admin.name],
        "password" : "bob"
      })
    }

    resource "vault_mount" "db" {
      path = "database"
      type = "database"
    }

    resource "vault_database_secret_backend_connection" "postgres" {
      backend           = vault_mount.db.path
      name              = "postgres"
      allowed_roles     = ["*"]
      verify_connection = false

      postgresql {
        connection_url = "postgres://username:password@localhost/database"
      }
    }
    $ terraform apply --auto-approve

then using bob to create a role associated to the `postgres` connection.

This patch changes the way the UI does the update: it still tries to
update the database connection but if it fails to do so because it does not
have the permission it just silently skip this part and updates the role.

This also update the error message returned to the user in case of issues
to include the actual errors.
@hashishaw hashishaw added the ui label Jan 3, 2024
Copy link
Contributor

@hellobontempo hellobontempo left a comment

Choose a reason for hiding this comment

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

Thanks for this improvement! I've made a couple minor comments, once the linting errors are resolved and the UI tests run this should be good to go!

@remilapeyre
Copy link
Contributor Author

Thanks for this improvement! I've made a couple minor comments, once the linting errors are resolved and the UI tests run this should be good to go!

Thanks, it should all be good now!

@hellobontempo hellobontempo added this to the 1.16.0-rc1 milestone Jan 5, 2024
Copy link
Contributor

@hellobontempo hellobontempo left a comment

Choose a reason for hiding this comment

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

Thank you for the thorough PR description and comments in the code. Beautiful work! ✨

@hellobontempo hellobontempo added backport/1.15.x Backport changes to `release/1.15.x` backport/1.14.x Backport changes to `release/1.14.x` labels Jan 5, 2024
@hellobontempo hellobontempo merged commit 3aee6ec into hashicorp:main Jan 5, 2024
61 of 62 checks passed
@hellobontempo hellobontempo modified the milestones: 1.16.0-rc1, 1.14.9 Jan 5, 2024
Monkeychip pushed a commit that referenced this pull request Jan 7, 2024
* Fix UI when editing database roles

When using a database role the UI will try to update the database connection
associated to the role. This is to make sure that the role is allowed to
use this connection:

    async _updateAllowedRoles(store, { role, backend, db, type = 'add' }) {
      const connection = await store.queryRecord('database/connection', { backend, id: db });
      const roles = [...connection.allowed_roles];
      const allowedRoles = type === 'add' ? addToArray([roles, role]) : removeFromArray([roles, role]);
      connection.allowed_roles = allowedRoles;
      return connection.save();
    },

    async createRecord(store, type, snapshot) {
      const serializer = store.serializerFor(type.modelName);
      const data = serializer.serialize(snapshot);
      const roleType = snapshot.attr('type');
      const backend = snapshot.attr('backend');
      const id = snapshot.attr('name');
      const db = snapshot.attr('database');
      try {
        await this._updateAllowedRoles(store, {
          role: id,
          backend,
          db: db[0],
        });
      } catch (e) {
        throw new Error('Could not update allowed roles for selected database. Check Vault logs for details');
      }

      return this.ajax(this.urlFor(backend, id, roleType), 'POST', { data }).then(() => {
        // ember data doesn't like 204s if it's not a DELETE
        return {
          data: assign({}, data, { id }),
        };
      });
    },

This is intended to help the administrator as the role will only work if
it is allowed by the database connection.

This is however an issue if the person doing the update does not have
the permission to update the connection: they will not be able to use
the UI to update the role even though they have the appropriate permissions
to do so (using the CLI or the API will work for example).

This is often the case when the database connections are created by a
centralized system but a human operator needs to create the roles.

You can try this with the following test case:

    $ cat main.tf
    resource "vault_auth_backend" "userpass" {
      type = "userpass"
    }

    resource "vault_generic_endpoint" "alice" {
      depends_on           = [vault_auth_backend.userpass]
      path                 = "auth/userpass/users/alice"
      ignore_absent_fields = true

      data_json = jsonencode({
        "policies" : ["root"],
        "password" : "alice"
      })
    }

    data "vault_policy_document" "db_admin" {
      rule {
        path         = "database/roles/*"
        capabilities = ["create", "read", "update", "delete", "list"]
      }
    }

    resource "vault_policy" "db_admin" {
      name   = "db-admin"
      policy = data.vault_policy_document.db_admin.hcl
    }

    resource "vault_generic_endpoint" "bob" {
      depends_on           = [vault_auth_backend.userpass]
      path                 = "auth/userpass/users/bob"
      ignore_absent_fields = true

      data_json = jsonencode({
        "policies" : [vault_policy.db_admin.name],
        "password" : "bob"
      })
    }

    resource "vault_mount" "db" {
      path = "database"
      type = "database"
    }

    resource "vault_database_secret_backend_connection" "postgres" {
      backend           = vault_mount.db.path
      name              = "postgres"
      allowed_roles     = ["*"]
      verify_connection = false

      postgresql {
        connection_url = "postgres://username:password@localhost/database"
      }
    }
    $ terraform apply --auto-approve

then using bob to create a role associated to the `postgres` connection.

This patch changes the way the UI does the update: it still tries to
update the database connection but if it fails to do so because it does not
have the permission it just silently skip this part and updates the role.

This also update the error message returned to the user in case of issues
to include the actual errors.

* Add changelog

* Also ignore error when deleting a role

* Address code review comments

---------

Co-authored-by: Chelsea Shaw <82459713+hashishaw@users.noreply.github.com>
@remilapeyre remilapeyre deleted the fix-ui-role branch January 7, 2024 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.14.x Backport changes to `release/1.14.x` backport/1.15.x Backport changes to `release/1.15.x` ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants