feat: Better error message for GitHub OIDC

[seebees]

Here is an example of what I was suggesting in #512

If a customer only configures a role and not accessKeyId then assuming the customer intent is to use this role is reasonable. If credentials could be derived in this case,
then we have introduced ambiguity into an authorization process. And if credentials can not be derived,
then we have created a very confusing customer experience.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[peterwoodworth]

@seebees I'm not finding much information on ACTIONS_ID_TOKEN_REQUEST_TOKEN environment variable. Can you explain and share documentation around what exactly this variable is for?

[seebees]

https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/about-security-hardening-with-openid-connect#updating-your-actions-for-oidc

I don't know everything :)
My understanding is that this variable is set,
along with ACTIONS_ID_TOKEN_REQUEST_URL
in GitHub actions hosted runners.
This information is used to connect to the OIDC provider in AWS (or wherever).

By primarily using this value as the switch for this behavior,
if customers do not set the correct permissions
they get error messages that are very difficult to debug.

[peterwoodworth]

I'm confused in how you're arriving to the conclusion that the lack of existence of this variable means that permissions have not been set. I came across this documentation before I posted my last comment, and I thought it was ultimately unclear exactly when the variable is set and what is contained inside it.

So, I went and tested this. Your code seems to successfully detect when permissions are unset. Additionally, testing Github Actions by running printenv as a step, it will show the presence of ACTIONS_ID_TOKEN_REQUEST_TOKEN if permissions are set and will not show the presence of this variable if permissions are not set. So, you do seem to be right

We have a problem however, in that this code breaks a test. This, in addition to documentation not being explicit about this environment variable, makes me hesitant to accept this solution. I need to do more research on this topic, but I can't promise I will get to it super soon. If you find any links to documentation or anything else which will clear up the purpose of this variable, let me know 🙂

[seebees]

This is great. I just tossed this together. I'm not terribly familiar with the codebase :)
What was at stake for me was getting a better onboarding experience.
Feel free to rip this apart or just close it and do something better.

[peterwoodworth]

Thanks again for this @seebees, and sorry for the delay here. We should definitely provide better error messaging when possible - we're going to reconsider how exactly we select which authentication method to use and will try to be clear when errors occur. Stay tuned for improvements on this in the future