Implement JENKINS-28010 Use Google Apps group for Authorization #18

juulderuysscher · 2020-01-07T13:06:25Z

I created an enhancement which makes it possible to fetch the Google Groups for an user via the Google Admin SDK.

https://developers.google.com/admin-sdk/directory/v1/reference/groups/list
https://issues.jenkins-ci.org/browse/JENKINS-28010

mjagielka · 2020-06-12T11:29:51Z

Hey. Any news here? Do you have any date when this feature will be released? It will be really useful.

lucasheringer · 2020-07-21T15:03:44Z

Any news here?
Do you have any date when this feature will be released?

jglick · 2020-08-05T14:37:05Z

Given that #17 remains unmerged I am guessing this plugin is abandoned. (@Vlatombe + @recampbell listed as having push permission)

jtnord · 2020-08-07T09:11:23Z

src/main/java/org/jenkinsci/plugins/googlelogin/GoogleOAuth2SecurityRealm.java

@@ -326,5 +421,20 @@ public FormValidation doCheckApiSecret(@QueryParameter String clientId, @QueryPa
        }
            */

+        @RequirePOST
+        public ListBoxModel doFillGsuiteServiceAccountCredentialsIdItems(@QueryParameter String serverUrl) {
+            Jenkins.getInstance().checkPermission(Jenkins.ADMINISTER);


I think this is incorrect.
if the user does not have admin (system read) then they should get a postbox with the current credential?

see https://github.com/jenkinsci/credentials-plugin/blob/master/docs/consumer.adoc for details

If they do not have admin they should not be configuring the security realm to begin with. I see nothing amiss here.

SYSTEM_READ allows rendering this but not configuring - and if the listbox does not contain the current credential it will be rednered incorrectly IIRC.

https://github.com/jenkinsci/credentials-plugin/blob/master/docs/consumer.adoc#providing-a-ui-form-element-to-let-a-user-select-credentials the first doFillXZ annotated example

If the plugin is using a new core dep, yes. Currently using an obsolete parent POM which does not even declare jenkins.version, so something old.

jtnord

I think there are som issues around the credential listbox population.
also won't work with system_read access

Vlatombe · 2020-08-10T14:03:11Z

src/main/java/org/jenkinsci/plugins/googlelogin/GoogleOAuth2SecurityRealm.java

+
+            do {
+                Groups groupsResult = googleAdminDirectoryService.groups().list()
+                        .setUserKey(email)


Shouldn't pageToken be used in this request?

Indeed .setPageToken(pageToken) should be used here. When the user has 200+ groups it will fail badly now.

oleg-filiutsich · 2021-05-28T21:23:14Z

Then the feature will release?

mayeco · 2021-08-17T15:59:29Z

ping

oleg-filiutsich · 2021-08-17T19:38:40Z

never00rei · 2021-08-19T08:15:45Z

Ping!!

mayeco · 2021-10-15T04:49:24Z

this is why cloudbees / jenkins is dying.

julien-deruere · 2021-10-15T12:44:12Z

I switched to github login instead, it works with github team perfectly.
If you can, do it!

mayerro · 2021-10-27T13:28:27Z

Ping!

jglick · 2021-10-29T16:41:38Z

Please do not add ping-type comments to issues. https://www.jenkins.io/doc/developer/plugin-governance/adopt-a-plugin/ if you want to do something about it.

antgel · 2021-10-30T11:08:47Z

@jglick Obviously you're right, obnoxious behaviour from people who don't understand nor care how open source works, nor do they care about everyone getting pinged every time they feel they have to add their useless comment when a thumbs-up on the original would presumably have done (I hope so anyway).

However, there is actually some code here, and nothing is happening. The link you sent includes the following:

Adopting abandoned plugins. Sometimes plugin maintainers move on without marking plugins for adoption, due to various reasons. In such case we can transfer ownership for plugins being hosted within the jenkinsci GitHub organization. Such transfer may take a while.

This plugin is already hosted within the jenkinsci GitHub organization. So now what? How do we even know who the maintainers are? What can the community expect from plugins hosted in the jenkinsci org?

never00rei · 2021-10-30T11:16:14Z

There is a precident here. It's not exactly fair to say "hey stop pinging" when the submitter and community is actually waiting for someone to ack this request. I for one would love to see this approved as I don't have to have custom code everywhere when this one plugin could actually deal with it.

…

On Sat, 30 Oct 2021, 12:08 Antony Gelberg, ***@***.***> wrote: @jglick <https://github.com/jglick> Obviously you're right, obnoxious behaviour from people who don't understand nor care how open source works, nor do they care about everyone getting pinged every time they feel they have to add their useless comment when a thumbs-up on the original would presumably have done (I hope so anyway). However, there is actually some code here, and nothing is happening. The link you sent includes the following: Adopting abandoned plugins. Sometimes plugin maintainers move on without marking plugins for adoption, due to various reasons. In such case we can transfer ownership for plugins being hosted within the jenkinsci GitHub organization. Such transfer may take a while. This plugin is already hosted within the jenkinsci GitHub organization. So now what? How do we even know who the maintainers are? What can the community expect from plugins hosted in the jenkinsci org? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#18 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACN2BMW52WSBAYGAKJHTGKLUJPN5XANCNFSM4KDZAZXQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

antgel · 2021-10-30T11:24:01Z

I think the point is that the useless comments are not the way to make progress.

jglick · 2021-11-01T21:59:54Z

How do we even know who the maintainers are?

You can look it up: https://github.com/jenkins-infra/repository-permissions-updater/blob/ba97350a9a74756f5e5f23780b40f146d1952647/permissions/plugin-google-login.yml#L8-L10 and if you want something shipped but a maintainer is not responsive, the page on jenkins.io documents how to propose taking over maintainership of the plugin (either with immediate approval from the old maintainer, or failing that, a two-week grace period IIRC).

In this case there actually is a maintainer review (#18 (comment)) which was never addressed. So if you care to move things along, best to file a subsuming PR which addresses any open review issues, or explain why they are misplaced.

antgel · 2021-11-02T09:35:49Z

Thanks @jglick , for me at least the comment got lost in the noise. @juulderuysscher Are you willing and able to address the comment? @Vlatombe I see you're active with the thumbs up, can you maybe elaborate on the comment for those of us who are new to Jenkins dev but would be happy to help out? Is the idea that there may be a lot of groups hence pagination is necessary?

Vlatombe · 2021-11-02T09:36:47Z

Is the idea that there may be a lot of groups hence pagination is necessary?

Yes exactly. Unfortunately I don't have a setup with google that would allow me to validate this change, but as it is, it is clear that pagination wouldn't work so the change isn't correct.

antgel · 2021-11-02T09:56:04Z

@Vlatombe Thanks! I'll see if we can take a look at that code and polish it, although we are new to Jenkins dev. For the avoidance of doubt, you're active and consider the plugin maintained?

juulderuysscher · 2021-11-02T10:32:35Z

Is the idea that there may be a lot of groups hence pagination is necessary?

Yes exactly. Unfortunately I don't have a setup with google that would allow me to validate this change, but as it is, it is clear that pagination wouldn't work so the change isn't correct.

Currently I don't have time to validate and test an improvement to this PR myself. pageToken should be fixed before merging this indeed.

Regarding the remarks on the Jenkins specific config at the time when I needed this it worked for me. However not sure how this can be improved? I'm not familiar with Jenkins internal APIs.

antgel · 2021-11-03T09:22:19Z

I've cloned the source so I can try to test and submit the pagination fix. The problem is that in order to test it, I run mvn hpi:run in accordance with the docs. This starts a local Jenkins at http://localhost:8080/jenkins/. However as expected, when I configure the credentials for the Google Login plugin, it complains because my URL isn't HTTPS. What's the sanest development flow? Build with mvn hpi:hpi and manually install on a Jenkins in the cloud that has HTTPS configured?

Scrap that, it works!

ArtemChekunov · 2022-01-17T14:29:55Z

any chance to get it merged?

antgel · 2022-03-16T21:37:18Z

@ArtemChekunov When I said "it works", I meant that I got the local Jenkins dev environment up and running. Sadly I haven't gotten further than that. Perhaps in coming weeks we may have time, but don't wait for me...

eugen-korentsov · 2023-06-06T14:27:47Z

Implement JENKINS-28010 Use Google Apps group for Authorization

139a00f

juulderuysscher force-pushed the feature/gsuite-groups branch from 8c2cdbc to 139a00f Compare January 7, 2020 13:31

juulderuysscher requested a review from recampbell January 17, 2020 12:18

jtnord reviewed Aug 7, 2020

View reviewed changes

Vlatombe reviewed Aug 10, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement JENKINS-28010 Use Google Apps group for Authorization #18

Implement JENKINS-28010 Use Google Apps group for Authorization #18

juulderuysscher commented Jan 7, 2020 •

edited

Loading

mjagielka commented Jun 12, 2020

lucasheringer commented Jul 21, 2020

jglick commented Aug 5, 2020 •

edited

Loading

jtnord Aug 7, 2020

jtnord Aug 7, 2020

jglick Aug 7, 2020

jtnord Aug 7, 2020 •

edited

Loading

jglick Aug 7, 2020

jtnord left a comment

Vlatombe Aug 10, 2020

juulderuysscher Nov 2, 2021

oleg-filiutsich commented May 28, 2021

mayeco commented Aug 17, 2021

oleg-filiutsich commented Aug 17, 2021

never00rei commented Aug 19, 2021

mayeco commented Oct 15, 2021

julien-deruere commented Oct 15, 2021 •

edited

Loading

mayerro commented Oct 27, 2021

jglick commented Oct 29, 2021

antgel commented Oct 30, 2021

never00rei commented Oct 30, 2021 via email

antgel commented Oct 30, 2021

jglick commented Nov 1, 2021

antgel commented Nov 2, 2021

Vlatombe commented Nov 2, 2021 •

edited

Loading

antgel commented Nov 2, 2021

juulderuysscher commented Nov 2, 2021 •

edited

Loading

antgel commented Nov 3, 2021 •

edited

Loading

ArtemChekunov commented Jan 17, 2022

antgel commented Mar 16, 2022

eugen-korentsov commented Jun 6, 2023

Implement JENKINS-28010 Use Google Apps group for Authorization #18

Are you sure you want to change the base?

Implement JENKINS-28010 Use Google Apps group for Authorization #18

Conversation

juulderuysscher commented Jan 7, 2020 • edited Loading

mjagielka commented Jun 12, 2020

lucasheringer commented Jul 21, 2020

jglick commented Aug 5, 2020 • edited Loading

jtnord Aug 7, 2020

Choose a reason for hiding this comment

jtnord Aug 7, 2020

Choose a reason for hiding this comment

jglick Aug 7, 2020

Choose a reason for hiding this comment

jtnord Aug 7, 2020 • edited Loading

Choose a reason for hiding this comment

jglick Aug 7, 2020

Choose a reason for hiding this comment

jtnord left a comment

Choose a reason for hiding this comment

Vlatombe Aug 10, 2020

Choose a reason for hiding this comment

juulderuysscher Nov 2, 2021

Choose a reason for hiding this comment

oleg-filiutsich commented May 28, 2021

mayeco commented Aug 17, 2021

oleg-filiutsich commented Aug 17, 2021

never00rei commented Aug 19, 2021

mayeco commented Oct 15, 2021

julien-deruere commented Oct 15, 2021 • edited Loading

mayerro commented Oct 27, 2021

jglick commented Oct 29, 2021

antgel commented Oct 30, 2021

never00rei commented Oct 30, 2021 via email

antgel commented Oct 30, 2021

jglick commented Nov 1, 2021

antgel commented Nov 2, 2021

Vlatombe commented Nov 2, 2021 • edited Loading

antgel commented Nov 2, 2021

juulderuysscher commented Nov 2, 2021 • edited Loading

antgel commented Nov 3, 2021 • edited Loading

ArtemChekunov commented Jan 17, 2022

antgel commented Mar 16, 2022

eugen-korentsov commented Jun 6, 2023

juulderuysscher commented Jan 7, 2020 •

edited

Loading

jglick commented Aug 5, 2020 •

edited

Loading

jtnord Aug 7, 2020 •

edited

Loading

julien-deruere commented Oct 15, 2021 •

edited

Loading

Vlatombe commented Nov 2, 2021 •

edited

Loading

juulderuysscher commented Nov 2, 2021 •

edited

Loading

antgel commented Nov 3, 2021 •

edited

Loading