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

Implement TFE API for Team Tokens #624

Merged
merged 13 commits into from
Oct 26, 2023

Conversation

tomwardill-payoneer
Copy link
Contributor

Towards #580

This is the initial API implementation for Team Tokens. The TFE integration tests pass, but I don't think the token can actually be used to do anything as yet.

I've added Team as a sort of half-concept for the RBAC. It's not a full category in it's own right, but uses the User team memberships to authorize. Not sure if that's desirable, but it seemed a minimal effort fit as I understand how the RBAC works.

Description string

// Token belongs to an team
Team string
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suspect this should be something closer to Team *team, otherwise there's no easy way to look up the Team details from the Token, without manually doing extra DB calls.

@leg100
Copy link
Owner

leg100 commented Oct 21, 2023

Hi Tom. Thanks for this. I know how difficult it is contributing to a codebase largely authored by someone else. But somehow you've managed to find your way through. Having multiple people familiar with the codebase, able to make large contributions such as this, shows promise for the project.

Yes, it makes sense to introduce team as a new RBAC "class". I think of RBAC as a Subject doing an Action on a Resource of a Class. And in this instance, a team (Subject) can create/read/get (Action) a token (Resource) for itself (Class). (The codebase doesn't refer to "Class" or something of that nature but it should do).

I've made the following updates:

  • Updated the "token middleware" to check for team token. This is the "missing link" to make a token actually do something.
  • Team token uses team ID not name. An ID is globally unique whereas a team name is unique only to an organization. The downside is that logging a team ID is not particuliarly meaningful for someone viewing the logs. But that's also an issue with workspaces etc, and logging generally could be much improved.
  • Team is now a Subject. The token middleware checks for a team token and returns the corresponding team. I've refactored out some of the RBAC logic from the User type and moved it into the Team type. After all, a user is mostly an aggregate of teams; a user can only do what its team membership allows it to do, with some exceptions (site admin).
  • The TFE docs state that you can only have one team token and if you attempt to create a token and one already exists, then it is replaced. I've updated the INSERT query to obey this behaviour.
  • No need to add UNIQUE to a primary key; primary keys in postgres are already unique. But as above, only one team token can exist per team, so I've added UNIQUE constraint to team_id.
  • Updated timestamp of migration to make it the latest migration. You may need to blat your database.
  • I've dropped the description attribute. Although the go-tfe type has a description attribute it doesn't seem to be used anywhere. Neither the TFE UI, nor the go-tfe tests, nor the tfe provider references the description. Feel free to re-introduce this if you have a need for it.

Please read through the changes and check that a team token passes authentication. I may have introduced regressions.

@tomwardill-payoneer
Copy link
Contributor Author

@leg100 I've rebased and updated to fix the conflicts.
Looks like the tests are passing for me, we shall see what Github says :)

@tomwardill-payoneer
Copy link
Contributor Author

@leg100 I think this is ready for merge/review.

Thanks so much for your earlier review and work here. I ran through some manual tests and fixed a SQL bit, but I think everything else is behaving.

@leg100 leg100 merged commit 1e4b173 into leg100:master Oct 26, 2023
5 checks passed
@leg100
Copy link
Owner

leg100 commented Oct 26, 2023

@tomwardill-payoneer Apologies for the delay in merging. I had to divert my attention to fixing bugs elsewhere. Thanks for your work on this!

leg100 added a commit that referenced this pull request Oct 27, 2023
🤖 I have created a release *beep* *boop*
---


## [0.1.15](v0.1.14...v0.1.15)
(2023-10-27)

### Features

* Implement TFE API for Team Tokens (#624) 1e4b173

### Bug Fixes

* fix local execution mode (#627) aefb365
* agent error reporting
([#628](#628))
([76e7dda](76e7dda))
* fixed defect with multiline tfvars not being escaped
([#631](#631))
([f35dffa](f35dffa))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Louis Garman <75728+leg100@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants