terraform-provider-github icon indicating copy to clipboard operation
terraform-provider-github copied to clipboard

Authorization fixes: v5.0.0 proposal

Open kfcampbell opened this issue 4 years ago • 10 comments

Authorization Superissue, v5.0.0 proposal

Recently we've had a lot of papercuts with the provider's authentication mechanism. I've performed a bunch of testing (see results below) and I've come up with a list of behavior that I think is broken in the current release that I think we can fix in the next major version.

Terraform Version

Terraform v1.0.11
on linux_amd64

Affected Resource(s)

Multiple resources: this issue concerns our authorization policies for the terraform provider

Terraform Configuration Files

Here's an example that creates a repository, used in attempting to reproduce #876.

terraform {
  required_providers {
    github = {
      source = "integrations/github"
      version = "4.18.0"
    }
  }
}

provider "github" {
  owner = "kfcampbell-terraform-provider"
  token = "ghp_personal_token_redacted"
}


resource "github_repository" "app_repo" {
  name        = "876-repro"
  description = "App repository"

  visibility = "private"
  delete_branch_on_merge = "true"
}

Behavior

See these issues for a non-comprehensive list of current authorization issues.

I have responded to a few of these threads (#876, #878, #655) with reproduction cases.

I've performed a pretty comprehensive test of authentication results

Release v5.0.0 proposed authorization changes:

  • Deprecate GITHUB_ORGANIZATION environment variable (and organization provider block configuration) in favor of GITHUB_OWNER
  • If no token is present, still process owner variables so that unauthenticated requests may be made against public org-owned repos
  • Improve error handling and messaging around missing tokens/owner configuration (currently it's pretty difficult to figure out this is a problem and act accordingly)
  • Tweaks to avoid pointless requests (for example, when no token or owner is configured, we try a pointless request to e.g. api.github.com//example-repo, which obviously won't succeed)
  • Docs overhaul:
    • Clarify how provider authentication works in the case where both the integrations and hashicorp providers are used (I keep returning to @tibbes's excellent comment here about it)
    • Specify in our docs exactly how the decision tree works for each form of authentication (app, provider PAT config, environment variable PAT config) and when/how they can be combined
  • Return an error in some cases where our requests fail but the provider doesn't return an error code (see scenarios described below)

What's next

I'm going to leave this issue open for a couple weeks and hope it inspires some community discussion! Barring major changes to the plan, I'll start work on an implementation in December, and updates will be published to this issue and repo, of course.

References

Related auth issues:

  • #878
  • #876
  • #655
  • #652
  • #647
  • #598

Authorization tests performed

  • no auth, readonly request
    • does not find an owner when an owner is set, and it should
      • the plan/apply succeed, and i don't think they should
    • does not find an owner when an organization is set, and it should
      • the plan/apply succeed, and i don't think they should
      • did get the deprecation notice, which is good
  • env var token set, no env var owner or org set
    • readonly request
      • infers owner of token as owner of repo (not correct in my case)
  • env var token set, env var owner set
    • readonly request
      • correct retrieves repository information
  • env var token set, env var org set to org
    • readonly request
      • correct retrieves repository information and displays deprecation notice
  • env var token set, env var org set to individual
    • readonly request
      • request fails since it does an org lookup first
      • however, terraform succeeds and i don't think it should
  • provider block owner set, no other config
    • readonly request fails and terraform succeeds with the usual 404
  • provider block owner and token set
    • readonly request
      • correctly returns repository information
  • provider block organization and token set
    • readonly request
      • correctly returns repository info and displays deprecation notice
  • provider block organization and owner, env var owner set
    • readonly request
      • env var config supersedes provider block config
  • provider block owner set, env var token set
    • readonly request
      • correctly returns repository info
  • provider block config is interchangeable (and interoperable) with env var config, and env var config is always dominant from what i can tell
  • app auth, no owner in any form set
    • readonly request
      • 404s on GET /user
  • app auth, env var owner set
    • both readonly and write requests work correctly
  • app auth, env var organization set
    • readonly request
      • correctly returns repository info and displays deprecation notice
  • app auth, env var token set
    • conflicts
  • app auth, provider token set
    • conflicts

kfcampbell avatar Nov 18 '21 19:11 kfcampbell

Also related is this: https://github.com/integrations/terraform-provider-github/issues/977

App auth tokens expire in 1hr with no renewal mechanism.

jcogilvie avatar Dec 15 '21 18:12 jcogilvie

+1 to this as I'm still seeing that implicit provider passing is broken.

SizZiKe avatar Jan 15 '22 00:01 SizZiKe

Any ETA for this? Since the authentication is kinda broken for organizations this really put stall to our GitHub management with Terraform.

Satak avatar Feb 09 '22 13:02 Satak

Same here. I can create repo's, but that's basically it. For all other resources, I get the dreaded This resource can only be used in the context of an organization, which makes this Provider pretty useless at this point in time.

alexdepalex avatar Feb 09 '22 22:02 alexdepalex

Same here. I'll be glad if you let me know ETA for this. :)

posquit0 avatar Mar 13 '22 18:03 posquit0

Apologies for the delay! This project isn't officially supported by GitHub; I've been doing it in my spare time. In the past month or so, I've made a concentrated effort to sell it to the organization as something we should support officially. That effort is still ongoing, and in the meantime I've been granted a couple hours a week to work on it. Velocity on the project should pick up a little bit, although I can't promise a date for this work yet.

kfcampbell avatar Mar 14 '22 16:03 kfcampbell

What is the current state of this provider? For me the app authentication is still broken and we need to use PAT.

Satak avatar Jul 08 '22 05:07 Satak

Still genuine interest here. @kfcampbell Appreciate you're working on this in your spare time.

alexdepalex avatar Jul 08 '22 05:07 alexdepalex

In my case, the issue was that I was passing my API URL, https://github.example/api/v3/ as the base URL rather than simply https://github.example/. This results in calls to e.g. https://github.example/api/v3/api/v3/orgs/... which obviously return 404. The provider ultimately spat out This resource can only be used in the context of an organization, "foo" is a user for a PAT and failed to create OAuth token from GitHub App: {"message":"Bad credentials","documentation_url":"https://docs.github.com/enterprise/3.5/rest"} for app auth.

I find the wording in the documentation particularly confusing:

This is the target GitHub base API endpoint.

which implies to me that I am responsible for appending api/v3/.

jrarmstro avatar Jul 27 '22 16:07 jrarmstro

v5.0.0 was released but this issue is not resolved. I was wondering if there are any updated plans for this issue. I think this issue should be resolved with a high priority.

I would like to thank you for investing your spare time to maintain this project! 😄

posquit0 avatar Sep 21 '22 05:09 posquit0

posquit0 avatar Apr 05 '23 17:04 posquit0

Hi! I'm sorry that this is still an issue. I haven't been able to make progress on any extended effort/investigation unfortunately. PRs are welcome, however!

kfcampbell avatar Apr 10 '23 22:04 kfcampbell

Should this be moved to the v6 milestone now? Also, there are many issues opened about authentication issues. Should those be linked to this one somehow to avoid duplication, and let everybody follow just this issue? 🤔

froblesmartin avatar Jun 02 '23 09:06 froblesmartin

We've stopped using milestones to plan releases for the provider recently, but you're correct we're already at v5 and so the title isn't valid anymore. I've changed the title to reflect this.

Also, there are many issues opened about authentication issues. Should those be linked to this one somehow to avoid duplication, and let everybody follow just this issue?

I'd like to use this as a tracking issue for auth upgrades and we can identify other issues as duplicates on a case-by-case basis.

kfcampbell avatar Jun 14 '23 18:06 kfcampbell

👋 Hey Friends, this issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!

github-actions[bot] avatar Mar 11 '24 01:03 github-actions[bot]