cla-assistant / cla-assistant

Contributor License Agreement assistant (CLA assistant)
https://cla-assistant.io
Apache License 2.0
1.33k stars 263 forks source link

Allowing employees of a CLA-certified organization to commit code #597

Closed sffc closed 3 years ago

sffc commented 4 years ago

The Unicode Consortium has CLAs on file from several companies. Currently, we import usernames into the cla-assistant.io dashboard, and we have to manually keep that list updated with reality. It might be nice if we could give cla-assistant a list of GitHub organizations whose employees are allowed to submit code, such that we don't have to manually maintain the list of individual usernames.

@srl295 @jefgen

ibakshay commented 4 years ago

Hi @sffc, thank you for reaching out to us. There is an allow list functionality in CLA Assistant and you can make use of it if you want to allow all the users from a GitHub organization to pass the CLA check.

Screen Shot 2020-07-13 at 21 12 10

srl295 commented 4 years ago

@ibakshay How are organizations considered? This sounds like it would only cover forks that were in the organization's repo, not all members of an organization forking to their own repo?

KharitonOff commented 4 years ago

This sounds like it would only cover forks that were in the organization's repo, not all members of an organization forking to their own repo?

Yes, this is how it works with "always allowed" organizations.

@tim-mc , do you think it would make sense to allow all members of the organization whether or not the fork is in the org or in the member's private space?

srl295 commented 4 years ago

^ @sffc @jefgen otherwise this would require the fork to be https://github.com/microsoft/icu (ok that works for @jefgen! ) but does not help https://github.com/jefgen/icu

silverhook commented 4 years ago

It might be nice if we could give cla-assistant a list of GitHub organizations whose employees are allowed to submit code, such that we don't have to manually maintain the list of individual usernames.

This would be a nice-to-have in our organisation as well.

The only concern I have – and I’m pretty sure, devs have thought about this – is when someone stops working for an organisation, but they were allowed to create a PR without the CLA check from their personal fork, while they were still at the org.

srl295 commented 4 years ago

It might be nice if we could give cla-assistant a list of GitHub organizations whose employees are allowed to submit code, such that we don't have to manually maintain the list of individual usernames.

This would be a nice-to-have in our organisation as well.

The only concern I have – and I’m pretty sure, devs have thought about this – is when someone stops working for an organisation, but they were allowed to create a PR without the CLA check from their personal fork, while they were still at the org.

This is a definite issue—off-boarding. Not everyone is quick to remove people from the public organizations. But, the discussion just above is not about personal forks but organizational forks. If you stop working for the organization, but are still allowed to create a PR under that organization, that's the fault of the organization for not properly removing your access.

KharitonOff commented 4 years ago

Another point to consider is that some people belong to multiple orgs. The question is, how to define in which org's name the contributor is acting as he creates a PR from his own fork?

silverhook commented 4 years ago

[…] If you stop working for the organization, but are still allowed to create a PR under that organization, that's the fault of the organization for not properly removing your access.

That’s for sure. And the onus can only be on the org itself here, the bot can’t second-guess whether an account is rightly or wrongly associated with the org.

This is a definite issue—off-boarding. Not everyone is quick to remove people from the public organizations. But, the discussion just above is not about personal forks but organizational forks.

In that case I misunderstood https://github.com/cla-assistant/cla-assistant/issues/597#issuecomment-658185092

To clarify, a nice-to-have for us would be that as long as I’m part of Liferay org, I don’t need to sign a CLA regardless if I create a PR from either https://github.com/liferay/ or https://github.com/silverhook/

Another point to consider is that some people belong to multiple orgs. The question is, how to define in which org's name the contributor is acting as he creates a PR from his own fork?

That is a good question, yes.

A similar consideration is org vs private – it may be that an org is whitelisted, but the contributor has been contributing not in their work/org time, but that their contribution is actually their own copyright (perhaps even that they have a contract where the code they contribute is theirs, not the company’s). Then again, that may be quite a rare issue.

binford2k commented 3 years ago

Another point to consider is that some people belong to multiple orgs. The question is, how to define in which org's name the contributor is acting as he creates a PR from his own fork?

This is already an unsolved problem. If I make a PR to a repo from my own fork, then sign off on it in the name of my company, then I go to another company and make another PR against the same repo, then it falsely believes I've already signed on behalf of the new company.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

sffc commented 3 years ago

@stalebot This is still a major pain point. Please don't close it.

octogonz commented 3 years ago

If I make a PR to a repo from my own fork, then sign off on it in the name of my company, then I go to another company and make another PR against the same repo, then it falsely believes I've already signed on behalf of the new company.

My group encountered this problem recently. Our case is very simple:

  1. We want to exempt our own employees from having to sign our own company's CLA
  2. Our employees are identified based on belonging to a GitHub organization
  3. CLA Assistant's feature for exempting an organization works fine, unless the PR is from a personal fork
  4. For an open source project, we prefer for people to use personal forks, so that they have the same experience as an external contributor.

Regarding #4, in the past I've seen cases where a bug was overlooked and caused friction for external contributors, because the project maintainers were not using forks, whereas external contributors were using forks.

The concern about "it falsely believes I've already signed on behalf of the new company" doesn't seem to be relevant to this scenario -- unless I'm missing something. (?)

Is the design that @silverhook proposed difficult to implement (maybe as an opt-in setting)? Or maybe there is some better way for us to exempt "everybody at our own company" (for example using a GitHub team name)?

Shegox commented 3 years ago

I fully agree that this would be a good feature to have, especially since it more or less allows "Company Contributor Licence Agreements (CCLA)" .

I think it makes sense to add it as an additional configuration field (instead of reusing the existing "Specify usernames to be always allowed") which is called something like "specify organizations which public members are always allowed to contribute". We can then do an additional API request to check if that member is a public member (as for private membership we would need more scopes from the signee and thats not really feasible). Optionally we can check as well for email domains (like @company.com) and accept all commits done by this domain as signed.

What do you think, would this cover your use case? If not, please add what you would like to see.

I will try (no guarantee however) if I can get a PoC working for this in the next time.

Could look in the configuration like this: image

srl295 commented 3 years ago

to check if that member is a public member (as for private membership we would need more scopes from the signee and thats not really feasible).

Makes sense to me.

An organization should do its own api calls to make sure its members have public membership. (I think GitHub ought to prompt you when you are added, but anyway. )

Edit: it's worth saying that, Also an org is responsible for removing members when they should be no longer have this access, that's not cla assistants job.

octogonz commented 3 years ago

I think it makes sense to add it as an additional configuration field (instead of reusing the existing "Specify usernames to be always allowed") which is called something like "specify organizations which public members are always allowed to contribute". We can then do an additional API request to check if that member is a public member (as for private membership we would need more scopes from the signee and thats not really feasible). Optionally we can check as well for email domains (like @company.com) and accept all commits done by this domain as signed.

What do you think, would this cover your use case?

@Shegox Yes, this would be awesome! It's exactly what I was envisioning. Thank you very much! (Let us know if you need any help with the implementation or testing.)

Optionally we can check as well for email domains (like @company.com) and accept all commits done by this domain as signed.

This probably wouldn't work for our scenario, because a lot of our employee commits are from emails like 4673363+octogonz@users.noreply.github.com, and (if I remember right) actions performed using the GitHub website always appear that way. Also our repos don't enforce commit.gpgsign, so a non-employee can put any arbitrary string in the email field.

But maybe someone else would find this option useful. (?)

it's worth saying that, Also an org is responsible for removing members when they should be no longer have this access, that's not cla assistants job.

Agreed. This is generally pretty easy to manage, at least for an organization with enough people to need this feature in the first place.

octogonz commented 3 years ago

BTW in discussing this, we realized that there is an extra benefit for exempting employees from signing the CLA:

If they don't sign while employed, then the CLA Assistant will prompt them to sign after they leave the company. This will remind them that new terms apply, now that the employment agreement has terminated. (Whereas if they had signed back when they were employed, then CLA Assistant will never prompt them again.)

Shegox commented 3 years ago

Got a prototype implementation working in #783. So the idea works, just need to separate it out into an additional config field in the UI and backend.

Shegox commented 3 years ago

The feature to exempt public members of an organization is now live on our staging system (https://k8s.cla-assistant.io/). If you have a spare organization (don't use your productive ones), you can test it out there. I will take some time this week to validate a few things and if everything looks good the change will hit production at the beginning of next week.

octogonz commented 3 years ago

@Shegox Thanks! We will test this and then follow up with the results. (I'm waiting on approval to authorize the staging service to access our repo.)

Shegox commented 2 years ago

Changes are now live in cla-assistant.io (production) as of the v2.10.0 release.

Have fun and let me know if anything weird happens :)

//cc @octogonz