8000 Add gimme-aws-creds assumer by jpts · Pull Request #810 · common-fate/granted · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add gimme-aws-creds assumer #810

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.

8000

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jan 30, 2025
Merged

Conversation

jpts
Copy link
Contributor
@jpts jpts commented Dec 2, 2024

What changed?

Add support for the gimme-aws-creds helper as an assumer, which supports AWS accounts integrated with Okta directly via SAML. The program supports multiple versions of Okta, MFA configurations etc. so we don't have to reimplement any of that logic.

On top of the basic support, it has the following features:

  • caches obtained temporary credentials via the secure storage mechanism
  • parses the gimme-aws-creds config file, so we can use other credential providers for non-matching profiles
  • limited support for headless credential_process based refresh. It depends on an opt in from the user (using either the --auto-login flag or config file setting), and a compatible version of the tool and Okta flow. It's opt-in because the user can't verify the device code, since it's hidden by the calling program.

Why?

Allows us to get credentials via Okta/SAML, which wasn't previously supported.

How did you test it?

granted compiled with go build -ldflags='-w -s -X github.com/common-fate/granted/internal/build.ConfigFolderName=.granted' ./cmd/granted

Tested over the last few months on multiple AWS accounts/orgs, including a mixed config with Identity Centre and other credential providers.

Potential risks

None

Is patch release candidate?

Yes, no breaking changes

Link to relevant docs PRs

Copy link
Contributor
@chrnorm chrnorm left a comment

Choose a reason for hiding this comment

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

Hey @jpts, thanks so much for the contribution! I've taken a look over it and have some initial feedback.

For context: long term, I'd actually like to remove the Assumer interface entirely. I believe that some of the third-party assumers are only used by one or two Granted users, and it seems like this interface adds complexity to Granted and additional code to maintain.

I'm thinking eventually that we'll replace this with some sort of hook - e.g. allow you to call a bash script which could then trigger gimme-aws-creds (or any other sort of login tooling).

In the meantime though I'd be happy to accept additional assumer contributions until this change is implemented. Your PR here is good because it doesn't affect any of the core session management code etc.

It might actually be better here if we could add a bash_assumer that would trigger a bash script - would this work to solve your use case? I'm just thinking that this might be a better step towards my long term goal of using hooks and eliminating the custom Assumer code altogether. Let me know if this would work for your use case though.

Separately I have a few comments specific to the code - I'll add these inline on your diff!

Copy link
Contributor
@chrnorm chrnorm left a comment

Choose a reason for hiding this comment

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

Added a few questions for you on the implementation!

@jpts
Copy link
Contributor Author
jpts commented Jan 20, 2025

@chrnorm thanks for the review and context around the direction of the tool. I've left my replies inline.

A new bash_assumer could work if it exposed the same sort of interfaces as the current assumer code. I don't think this assumer is particularly unique or special, so a generic hook/integration should work well.

Copy link
Contributor
@chrnorm chrnorm left a comment

Choose a reason for hiding this comment

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

Thanks @jpts, just the comment on the additional config flag is outstanding and once that is resolved this will be ready to merge!

@jpts jpts requested a review from chrnorm January 22, 2025 10:05
@chrnorm
Copy link
Contributor
chrnorm commented Jan 30, 2025

Thanks @jpts!

@chrnorm chrnorm merged commit 2add2a1 into common-fate:main Jan 30, 2025
@jpts jpts deleted the add-gimme-aws-creds-assumer branch February 25, 2025 17:24
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Mar 2, 2025
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [common-fate/granted](https://github.com/common-fate/granted) | minor | `v0.36.3` -> `v0.38.0` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>common-fate/granted (common-fate/granted)</summary>

### [`v0.38.0`](https://github.com/common-fate/granted/releases/tag/v0.38.0)

[Compare Source](common-fate/granted@v0.37.0...v0.38.0)

#### What's Changed

-   Bump golang.org/x/crypto from 0.25.0 to 0.31.0 by [@&#8203;dependabot](https://github.com/dependabot) in common-fate/granted#815
-   Add gimme-aws-creds assumer by [@&#8203;jpts](https://github.com/jpts) in common-fate/granted#810
-   Bump golang.org/x/net from 0.26.0 to 0.33.0 by [@&#8203;dependabot](https://github.com/dependabot) in common-fate/granted#824
-   Make leading/trailing spaces optional in templating by [@&#8203;agershman](https://github.com/agershman) in common-fate/granted#792
-   Fix golangci-lint CI failure by [@&#8203;jpts](https://github.com/jpts) in common-fate/granted#832
-   Update service_map.go by [@&#8203;wayne-folkes](https://github.com/wayne-folkes) in common-fate/granted#829
-   Bump github.com/go-jose/go-jose/v4 from 4.0.2 to 4.0.5 by [@&#8203;dependabot](https://github.com/dependabot) in common-fate/granted#831
-   GHA: upgrade upload-artifact version by [@&#8203;chrnorm](https://github.com/chrnorm) in common-fate/granted#833

#### New Contributors

-   [@&#8203;agershman](https://github.com/agershman) made their first contribution in common-fate/granted#792

**Full Changelog**: common-fate/granted@v0.37.0...v0.38.0

### [`v0.37.0`](https://github.com/common-fate/granted/releases/tag/v0.37.0)

[Compare Source](common-fate/granted@v0.36.3...v0.37.0)

#### Changelog

-   [`91e5e76`](common-fate/granted@91e5e76) Josh/cf 3767 add cli flag to attach jira ticket in assume ([#&#8203;808](common-fate/granted#808))
-   [`e047104`](common-fate/granted@e047104) Service flag support for AWS Backup ([#&#8203;804](common-fate/granted#804))

Download this release by following our [getting started guide](https://docs.commonfate.io/granted/getting-started).

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4xODIuMyIsInVwZGF0ZWRJblZlciI6IjM5LjE4Mi4zIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
@set92
Copy link
set92 commented Mar 23, 2025

I'm missing some documentation, or tutorial on how to use this, right?

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.

3 participants
0