Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add handler to check ownership of repo or org #13

Merged
merged 3 commits into from
Mar 26, 2021

Conversation

JasperTimm
Copy link

This PR covers: #10

Given a Github user ID and an ID for a Repository or Organization, the new handler checks whether the user is either:

  • The owner of the Repository itself
  • A member of the Organization given
  • A member of the Organization which owns the Repository

The result is simply a "true" or "false" string for now, but we can obviously add more info in the response object if needed.

NOTE: We're only checking membership in the Organization for now as you need to be a member of an Organization to see what everyone's roles are. Hoping to fix this later.

Also, the version on the @octobay/adapters lib was old and things were sort of breaking, so this was updated.

@mktcode mktcode enabled auto-merge March 25, 2021 18:04
Copy link
Contributor

@mktcode mktcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you could just change the response type to boolean I think we're good here for now.

const ownerType = nodeType == "Repository" ? response.data.node.owner.__typename : null

if (nodeType == "Repository" && ownerType == "User") {
const result = response.data.node.owner.id == githubUserId ? "true" : "false"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be an actual boolean. Makes it more easy on the contract side as well.

if (response.data.node.__typename != "User") {
callback(500, Requester.errored(jobRunID, { checkOwnershipError: `Node (${githubUserId}) is not a User.` }))
} else {
const result = response.data.node.organization ? "true" : "false"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here then

],
"tasks": [
{ "type": "check-ownership" },
{ "type": "ethbytes32" },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here

auto-merge was automatically disabled March 26, 2021 15:03

Head branch was pushed to by a user without write access

@JasperTimm
Copy link
Author

@mktcode - fixed

@mktcode mktcode merged commit 2468381 into OctobayArchive:main Mar 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants