Collect HTTP Headers from Coursier for authentication #2533

markvandertol · 2022-02-20T17:11:57Z

This PR lets Scala Steward also pick up custom headers used for authentication. This is for example needed for Gitlab. I think looking at csrConfiguration.value.authenticationByRepositoryId is the cleanest and most generic way to achieve this.

Example snippet of a .sbt file from a project that sets a custom HTTP-header for authentication with Coursier:

resolvers += "gitlab-repo" at "https://gitlab.example.com/api/v4/groups/1/-/packages/maven",
csrConfiguration ~= _.addRepositoryAuthentication("gitlab-repo", Authentication(Seq( ("private-token", "xxx") )))

It works for SBT and Mill, but doesn't work for Maven

codecov · 2022-02-21T08:13:04Z

Codecov Report

Merging #2533 (ffd36e5) into main (fcd5ea3) will decrease coverage by 0.67%.
The diff coverage is 34.09%.

@@            Coverage Diff             @@
##             main    #2533      +/-   ##
==========================================
- Coverage   81.32%   80.65%   -0.68%     
==========================================
  Files         146      146              
  Lines        2592     2621      +29     
  Branches       43       45       +2     
==========================================
+ Hits         2108     2114       +6     
- Misses        484      507      +23

Impacted Files	Coverage Δ
.../scala/org/scalasteward/core/application/Cli.scala	`100.00% <ø> (ø)`
...a/org/scalasteward/mill/plugin/StewardPlugin.scala	`0.00% <0.00%> (ø)`
...la/org/scalasteward/sbt/plugin/StewardPlugin.scala	`0.00% <0.00%> (ø)`
...a/org/scalasteward/core/coursier/CoursierAlg.scala	`71.11% <35.71%> (-14.19%)`	⬇️
.../org/scalasteward/core/buildtool/mill/parser.scala	`66.66% <50.00%> (-1.52%)`	⬇️
...org/scalasteward/core/buildtool/maven/parser.scala	`100.00% <100.00%> (ø)`
...in/scala/org/scalasteward/core/data/Resolver.scala	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

julienrf

There are a couple of minor details that could be discussed (e.g., the usage of Option[List[Header]] vs List[Header], and the way headers are serialized in JSON — I would just write ["foo", "bar"] instead of { "key": "foo", "value": "bar" }), but overall it looks correct to me, and it would unblock all the people that use a gitlab package registry!

markvandertol · 2022-03-05T08:16:14Z

Thanks for having a look. I replaced the Option[List[Header]] with List[Header]. That indeed simplifies the code.
I feel that encoding { "key": "foo", "value": "bar" } as object instead of array is clearer, the same as the credentials object is encoded as { "user": "foo", "pass", "bar" }. Also, because you could have more than one header. If you want I can change this.

julienrf · 2022-03-11T09:08:44Z

May I kindly request a review from the maintainers?

exoego · 2022-03-11T09:16:17Z

modules/core/src/main/scala/org/scalasteward/core/coursier/CoursierAlg.scala

+    } else {
+      Some(
+        new Authentication(
+          credentials.fold("")(_.user),


Q) Does "" for user works ?

If not, error message like "Authentication error: missing user name" might help.

Yes, that what's Coursier also does internally when just specifying headers. See: https://github.com/coursier/coursier/blob/2cb552ea934a100f52ce79f94278304c90d808a4/modules/util/shared/src/main/scala/coursier/core/Authentication.scala#L88

modules/core/src/test/scala/org/scalasteward/core/update/PruningAlgTest.scala

exoego

LGTM
Great job!!

I will wait other maintainers for a week.

cipriansofronia · 2022-03-18T07:29:26Z

@markvandertol, could u also update the gitlab docs on how we can use this new feature?

markvandertol · 2022-04-03T17:05:35Z

@markvandertol, could u also update the gitlab docs on how we can use this new feature?

@cipriansofronia I've added a few lines explaining how to authenticate against a Gitlab repository. Does this clarify enough how it works? No specific code for Scala Steward is needed.

exoego · 2022-04-04T01:18:28Z

docs/running.md

+Scala Steward is compatible with Coursier authentication using headers. To authenticate 
+using the [Gitlab CI/CD job token](https://docs.gitlab.com/ee/ci/jobs/ci_job_token.html), while also supporting your own private token when performing


jchapuis · 2022-04-05T14:28:27Z

Looking forward to this one being released, thanks a lot @markvandertol

cipriansofronia · 2022-04-07T08:20:10Z

@fthomas can this PR get some ❤️ from you? 😇

markvandertol · 2022-04-13T07:14:29Z

Is there anything preventing this PR from being merged? 🙂

exoego · 2022-04-15T11:47:46Z

@fthomas Do you have a time to test this with your test Steward instance ??

987Nabil · 2022-04-15T11:54:49Z

modules/sbt-plugin/src/main/scala/org/scalasteward/sbt/plugin/StewardPlugin.scala

+              csrConfiguration.value.authenticationByRepositoryId.toList
+            if repositoryId == name
+            (headerKey, headerValue) <- authentication.headers
+          } yield Resolver.Header(headerKey, headerValue)


I think you should not only take the headers from the csrConfiguration but also username and password.
Setting these via csrConfiguration is not wrong. And I would prefer csrConfiguration over credentials if present, since it is more specific/explicit.

Funny enough, I did not see this pr and coded it myself that way. Maybe you can have a look :)

I forgot the @markvandertol :)

I am going to merge as-is since markvandertol seems busy now.
@987Nabil You can submit a PR for that change 😉

Sorry for not replying to this earlier. It indeed looks more consistent, but I'm not entirely sure how large of a breaking change would be, e.g. for people who disabled coursier in their projects. Retrieving the credentials old way, but looking for headers in csrConfiguration shouldn't be breaking, so I propose to do that in a different PR so the impact can better be assessed.

We can do this in another PR for sure. But I think, deactivating coursier and then relying on csrConfiguration where csr = coursier and the description in sbt also explicitly mentions coursier, seems to be odd.

jchapuis · 2022-05-09T06:30:39Z

👋 is there anything holding up the merge at this point?

kaffepanna · 2022-05-12T12:27:00Z

wave is there anything holding up the merge at this point?

I am wondering that to. This would really be a good thing to have when managing dependencies in private gilab repos

987Nabil · 2022-05-12T16:16:14Z

wave is there anything holding up the merge at this point?

I am wondering that to. This would really be a good thing to have when managing dependencies in private gilab repos

It is not only gitlb. Also Google Cloud Storage or Artifactregistry or managing packages in AWS need this feature.

exoego · 2022-06-15T01:57:19Z

@markvandertol Sorry for waiting so long. Could you resolve the conflict? 🙇

markvandertol · 2022-06-15T07:01:35Z

No worries. I resolved the merge conflict. 🙂

michaltomanski · 2022-07-06T08:08:46Z

Thanks Mark for this PR! It looks promising.

Any chance to have it merged some time soon?

fthomas · 2022-07-14T13:07:24Z

Thanks for having a look. I replaced the Option[List[Header]] with List[Header]. That indeed simplifies the code.

Having the fields optional allows to read in persisted RepoCaches of previous Scala Steward runs. If the header fields are non-optional, Scala Steward fails to read these files:

[info] 2022-07-14 14:48:58,983 INFO  ──────────── Steward scala-steward-org/scala-steward ────────────
[info] 2022-07-14 14:48:58,983 INFO  Check cache of scala-steward-org/scala-steward
[info] 2022-07-14 14:48:59,080 ERROR Failed to parse or decode JSON from /home/frank/data/code/scala-steward/core/workspace/store/repo_cache/v1/github/scala-steward-org/scala-steward/repo_cache.json
[info] io.circe.DecodingFailure$$anon$2: Attempt to decode value on failed cursor: DownField(headers),DownField(MavenRepository),DownArray,DownField(resolvers),DownArray,DownField(dependencyInfos)

Another alternative would be to bump the version of the repoCacheStore here. But for this small change I would prefer making the fields optional.

Other than that, this PR looks good to me.

markvandertol · 2022-08-08T21:01:56Z

Thanks @fthomas for having a look at the PR.

I fixed the issue by using a default value of Nil for the headers, and using deriveConfiguredCodec as shown below.

  @annotation.nowarn("cat=unused-locals")
  implicit val resolverCodec: Codec[Resolver] = {
    implicit val customConfig: Configuration = Configuration.default.withDefaults
    deriveConfiguredCodec
  }

I had to suppress the warning, as the implicit is only used by a macro. I could also have used the compiler flag -Ywarn-macros:after, but that would be inconsistent with other @annotation.nowarn usages.

exoego · 2022-08-11T11:50:12Z

Sorry for the late response.

jchapuis · 2022-10-07T08:41:20Z

Any chance this can get shipped in a new release? 😃 🙏 thanks!

fthomas · 2022-12-21T19:39:16Z

Just noticed that this change broke StewardPlugin.scala for old sbt versions. See #2847 for details.

markvandertol force-pushed the coursier-headers branch from db79b9d to 251b1f1 Compare February 21, 2022 07:54

markvandertol mentioned this pull request Mar 4, 2022

Attach credentials as http headers required for Gitlab's package registry authentication #2465

Closed

julienrf reviewed Mar 4, 2022

View reviewed changes

markvandertol force-pushed the coursier-headers branch from 251b1f1 to 31205be Compare March 5, 2022 08:12

exoego reviewed Mar 11, 2022

View reviewed changes

modules/core/src/test/scala/org/scalasteward/core/update/PruningAlgTest.scala Show resolved Hide resolved

markvandertol force-pushed the coursier-headers branch from 31205be to d63d865 Compare March 12, 2022 08:22

exoego approved these changes Mar 14, 2022

View reviewed changes

markvandertol force-pushed the coursier-headers branch from d63d865 to 62de957 Compare April 3, 2022 17:02

exoego reviewed Apr 4, 2022

View reviewed changes

markvandertol force-pushed the coursier-headers branch from 62de957 to 2fd7178 Compare April 4, 2022 06:46

987Nabil reviewed Apr 15, 2022

View reviewed changes

987Nabil mentioned this pull request Apr 15, 2022

Use coursier repo specific authentication settings #2575

Closed

exoego added the enhancement New feature or request label Jun 15, 2022

markvandertol force-pushed the coursier-headers branch from 2fd7178 to c806d3b Compare June 15, 2022 06:58

Collect HTTP Headers from Coursier for authentication

ffd36e5

markvandertol force-pushed the coursier-headers branch from c806d3b to ffd36e5 Compare August 8, 2022 20:57

exoego added this to the 0.16.0 milestone Aug 11, 2022

exoego merged commit 9df61a1 into scala-steward-org:main Aug 11, 2022

exoego mentioned this pull request Aug 11, 2022

Issues parsing mill modules headers #2692

Closed

fthomas mentioned this pull request Dec 21, 2022

StewardPlugin.scala cannot be loaded by sbt < 1.3.0 #2847

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Collect HTTP Headers from Coursier for authentication #2533

Collect HTTP Headers from Coursier for authentication #2533

markvandertol commented Feb 20, 2022

codecov bot commented Feb 21, 2022 •

edited

Loading

julienrf left a comment

markvandertol commented Mar 5, 2022

julienrf commented Mar 11, 2022

exoego Mar 11, 2022 •

edited

Loading

markvandertol Mar 11, 2022

exoego left a comment

cipriansofronia commented Mar 18, 2022

markvandertol commented Apr 3, 2022 •

edited

Loading

exoego Apr 4, 2022

jchapuis commented Apr 5, 2022

cipriansofronia commented Apr 7, 2022

markvandertol commented Apr 13, 2022

exoego commented Apr 15, 2022

987Nabil Apr 15, 2022 •

edited

Loading

987Nabil Apr 15, 2022

exoego Jun 15, 2022

markvandertol Jun 15, 2022

987Nabil Jun 15, 2022

jchapuis commented May 9, 2022

kaffepanna commented May 12, 2022

987Nabil commented May 12, 2022

exoego commented Jun 15, 2022

markvandertol commented Jun 15, 2022

michaltomanski commented Jul 6, 2022

fthomas commented Jul 14, 2022 •

edited

Loading

markvandertol commented Aug 8, 2022

exoego commented Aug 11, 2022

jchapuis commented Oct 7, 2022 •

edited

Loading

fthomas commented Dec 21, 2022

		Scala Steward is compatible with Coursier authentication using headers. To authenticate
		using the [Gitlab CI/CD job token](https://docs.gitlab.com/ee/ci/jobs/ci_job_token.html), while also supporting your own private token when performing

Collect HTTP Headers from Coursier for authentication #2533

Collect HTTP Headers from Coursier for authentication #2533

Conversation

markvandertol commented Feb 20, 2022

codecov bot commented Feb 21, 2022 • edited Loading

Codecov Report

julienrf left a comment

Choose a reason for hiding this comment

markvandertol commented Mar 5, 2022

julienrf commented Mar 11, 2022

exoego Mar 11, 2022 • edited Loading

Choose a reason for hiding this comment

markvandertol Mar 11, 2022

Choose a reason for hiding this comment

exoego left a comment

Choose a reason for hiding this comment

cipriansofronia commented Mar 18, 2022

markvandertol commented Apr 3, 2022 • edited Loading

exoego Apr 4, 2022

Choose a reason for hiding this comment

jchapuis commented Apr 5, 2022

cipriansofronia commented Apr 7, 2022

markvandertol commented Apr 13, 2022

exoego commented Apr 15, 2022

987Nabil Apr 15, 2022 • edited Loading

Choose a reason for hiding this comment

987Nabil Apr 15, 2022

Choose a reason for hiding this comment

exoego Jun 15, 2022

Choose a reason for hiding this comment

markvandertol Jun 15, 2022

Choose a reason for hiding this comment

987Nabil Jun 15, 2022

Choose a reason for hiding this comment

jchapuis commented May 9, 2022

kaffepanna commented May 12, 2022

987Nabil commented May 12, 2022

exoego commented Jun 15, 2022

markvandertol commented Jun 15, 2022

michaltomanski commented Jul 6, 2022

fthomas commented Jul 14, 2022 • edited Loading

markvandertol commented Aug 8, 2022

exoego commented Aug 11, 2022

jchapuis commented Oct 7, 2022 • edited Loading

fthomas commented Dec 21, 2022

codecov bot commented Feb 21, 2022 •

edited

Loading

exoego Mar 11, 2022 •

edited

Loading

markvandertol commented Apr 3, 2022 •

edited

Loading

987Nabil Apr 15, 2022 •

edited

Loading

fthomas commented Jul 14, 2022 •

edited

Loading

jchapuis commented Oct 7, 2022 •

edited

Loading