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

storage: acl: refactor #360

Merged

Conversation

stephenplusplus
Copy link
Contributor

Fixes #351

This introduces support for a nicer API to ACL manipulations. The new methods are broken into groups representing a permission level, owners, readers, and writers. Each group has access methods to add or delete various entities: allAuthenticatedUsers, allUsers, domains, groups, projects, and users.

The full list of methods:

var acl = (myFile || myBucket).(acl || acl.default).(owners || readers || writers);

acl.addAllAuthenticatedUsers(function(err, aclObject) {});
acl.deleteAllAuthenticatedUsers(function(err) {});

acl.addAllUsers(function(err, aclObject) {});
acl.deleteAllUsers(function(err) {});

acl.addDomain(function(err, aclObject) {});
acl.deleteDomain(function(err) {});

acl.addGroup(function(err, aclObject) {});
acl.deleteGroup(function(err) {});

acl.addProject(function(err, aclObject) {});
acl.deleteProject(function(err) {});

acl.addUser(function(err, aclObject) {});
acl.deleteUser(function(err) {});

Additionally, a couple methods were added to simplify common operations; making a bucket or file publicly readable or the inverse.

(myFile || myBucket).makePrivate(function(err) {});
(myFile || myBucket).makePublic(function(err) {});

This will run setMetadata with a predefinedAcl value set to private or publicRead.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 21, 2015
* Transform API responses to a consistent object format.
*
* @private
*/
Acl.prototype.makeAclObject_ = function(accessControlObject) {
var obj = {
scope: accessControlObject.scope,
scope: accessControlObject.entity,

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@stephenplusplus stephenplusplus added the api: storage Issues related to the Cloud Storage API. label Jan 21, 2015
@ryanseys
Copy link
Contributor

hmm... do you like delete or remove more? I'm feeling remove more because acl.writers.removeUser sounds more realistic than acl.writers.deleteUser but because we already have delete it might make sense to keep it that way... tough call.

@stephenplusplus
Copy link
Contributor Author

I prefer delete for consistency across our library. Files, buckets, datasets, etc. are all deleted, and as you pointed out, we have acl.delete as well.

@ryanseys
Copy link
Contributor

Works for me! I've been writing too much ruby recently.

@stephenplusplus
Copy link
Contributor Author

ha_ha()

@ryanseys
Copy link
Contributor

ha_ha()

No brackets necessary.

@stephenplusplus
Copy link
Contributor Author

I think we have PR review inception here.


that[accessMethod]({
scope: scope,
role: role

This comment was marked as spam.

This comment was marked as spam.

@ryanseys
Copy link
Contributor

Looks good to me :)

* @alias acl.makePublic
*
* @example
* myBucket.acl.makePublic(function(err, aclObject) {});

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@ryanseys
Copy link
Contributor

I think this is acceptable for merge. Should probably wait for sign-off by @jgeewax for the makePrivate and makePublic however.

@stephenplusplus
Copy link
Contributor Author

Updated makePublic() and makePrivate(): stephenplusplus@fd35f27

They are now on the File and Bucket object themselves.

@ryanseys
Copy link
Contributor

Is this ready for another look?

@stephenplusplus
Copy link
Contributor Author

Yes, and I should have explained the new makePublic() and makePrivate() methods use a shorthand "predefinedAcl" value that Ryan found. When a user makes an object public or private, it updates the "predefinedAcl" value as a metadata request to publicRead or private.

@@ -20,7 +20,8 @@

var Acl = require('../../lib/storage/acl.js');
var assert = require('assert');
var storage = require('../../lib/storage/index.js');
var async = require('async');
var Storage = require('../../lib/storage/index.js');

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@ryanseys
Copy link
Contributor

@stephenplusplus For now, you should just remove makePublic and makePrivate and we can land these changes. Those methods are going to be more troublesome and are special cases and thus should be treated separately.

@stephenplusplus
Copy link
Contributor Author

Sounds good. 👍

@jgeewax
Copy link
Contributor

jgeewax commented Jan 27, 2015

Cool - can we create a ticket to make sure we don't forget those two methods?

@ryanseys
Copy link
Contributor

@jgeewax Done: #364

@stephenplusplus
Copy link
Contributor Author

This is now updated to remove makePublic() and makePrivate() as well as change scope -> entity.

this.makeReq = options.makeReq;
this.pathPrefix = options.pathPrefix;
}

/**
* A convenience object of methods to add or delete ACL permissions from an

This comment was marked as spam.

sofisl pushed a commit that referenced this pull request Nov 11, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@types/mocha](https://github.com/DefinitelyTyped/DefinitelyTyped) | devDependencies | major | [`^7.0.0` -> `^8.0.0`](https://renovatebot.com/diffs/npm/@types%2fmocha/7.0.2/8.0.0) |

---

### Renovate configuration

:date: **Schedule**: "after 9am and before 3pm" (UTC).

:vertical_traffic_light: **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

:recycle: **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.

:no_bell: **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#googleapis/nodejs-cloud-container).
sofisl pushed a commit that referenced this pull request Nov 11, 2022
* feat: monitoring conversion by delete js file and add ts from generator

* upgrade package.json

* fixed samples test by modify the matched path

* prelint, add eslint for samples test

* fix upstream proto links, get rid of synth hack

Co-authored-by: Benjamin E. Coe <bencoe@google.com>
sofisl pushed a commit that referenced this pull request Nov 11, 2022
sofisl pushed a commit that referenced this pull request Nov 11, 2022
gcr.io/repo-automation-bots/owlbot-nodejs:latest@sha256:f93bb861d6f12574437bb9aee426b71eafd63b419669ff0ed029f4b7e7162e3f
sofisl pushed a commit that referenced this pull request Nov 11, 2022
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [jsdoc-fresh](https://github.com/googleapis/jsdoc-fresh) | [`^1.0.1` -> `^2.0.0`](https://renovatebot.com/diffs/npm/jsdoc-fresh/1.1.1/2.0.0) | [![age](https://badges.renovateapi.com/packages/npm/jsdoc-fresh/2.0.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/jsdoc-fresh/2.0.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/jsdoc-fresh/2.0.0/compatibility-slim/1.1.1)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/jsdoc-fresh/2.0.0/confidence-slim/1.1.1)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>googleapis/jsdoc-fresh</summary>

### [`v2.0.0`](https://github.com/googleapis/jsdoc-fresh/blob/HEAD/CHANGELOG.md#&#8203;200-httpsgithubcomgoogleapisjsdoc-freshcomparev111v200-2022-05-18)

[Compare Source](https://github.com/googleapis/jsdoc-fresh/compare/v1.1.1...v2.0.0)

##### ⚠ BREAKING CHANGES

-   update library to use Node 12 ([#&#8203;108](https://github.com/googleapis/jsdoc-fresh/issues/108))

##### Build System

-   update library to use Node 12 ([#&#8203;108](https://github.com/googleapis/jsdoc-fresh/issues/108)) ([e61c223](https://github.com/googleapis/jsdoc-fresh/commit/e61c2238db8900e339e5fe7fb8aea09642290182))

##### [1.1.1](https://www.github.com/googleapis/jsdoc-fresh/compare/v1.1.0...v1.1.1) (2021-08-11)

##### Bug Fixes

-   **build:** migrate to using main branch ([#&#8203;83](https://www.github.com/googleapis/jsdoc-fresh/issues/83)) ([9474adb](https://www.github.com/googleapis/jsdoc-fresh/commit/9474adbf0d559d319ff207397ba2be6b557999ac))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 9am and before 3pm" (UTC), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/nodejs-datalabeling).
sofisl pushed a commit that referenced this pull request Nov 11, 2022
Update .repo-metadata.json as required by go/library-data-integrity
sofisl pushed a commit that referenced this pull request Nov 11, 2022
* chore(main): release 4.1.1

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
sofisl pushed a commit that referenced this pull request Nov 11, 2022
This PR was generated using Autosynth. 🌈

Synth log will be available here:
https://source.cloud.google.com/results/invocations/cc99acfa-05b8-434b-9500-2f6faf2eaa02/targets

- [ ] To automatically regenerate this PR, check this box.

Source-Link: googleapis/synthtool@799d8e6
sofisl pushed a commit that referenced this pull request Nov 11, 2022
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
sofisl pushed a commit that referenced this pull request Nov 18, 2022
This PR was generated using Autosynth. 🌈

Synth log will be available here:
https://source.cloud.google.com/results/invocations/cb5a7bf7-f080-4698-bd24-ff5880d64fc8/targets

- [ ] To automatically regenerate this PR, check this box.

PiperOrigin-RevId: 361273630
Source-Link: googleapis/googleapis@5477122
sofisl pushed a commit that referenced this pull request Nov 30, 2022
[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [sinon](https://sinonjs.org/) ([source](https://github.com/sinonjs/sinon)) | [`^13.0.0` -> `^14.0.0`](https://renovatebot.com/diffs/npm/sinon/13.0.2/14.0.0) | [![age](https://badges.renovateapi.com/packages/npm/sinon/14.0.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/sinon/14.0.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/sinon/14.0.0/compatibility-slim/13.0.2)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/sinon/14.0.0/confidence-slim/13.0.2)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>sinonjs/sinon</summary>

### [`v14.0.0`](https://github.com/sinonjs/sinon/blob/HEAD/CHANGES.md#&#8203;1400)

[Compare Source](https://github.com/sinonjs/sinon/compare/v13.0.2...v14.0.0)

-   [`c2bbd826`](https://github.com/sinonjs/sinon/commit/c2bbd82641444eb5b32822489ae40f185afbbf00)
    Drop node 12 (Morgan Roderick)
    > And embrace Node 18
    >
    > See https://nodejs.org/en/about/releases/

*Released by Morgan Roderick on 2022-05-07.*

</details>

---

### Configuration

📅 **Schedule**: "after 9am and before 3pm" (UTC).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/nodejs-datacatalog).
sofisl pushed a commit that referenced this pull request Sep 14, 2023
This PR was generated using Autosynth. 🌈


<details><summary>Log from Synthtool</summary>

```
2020-03-22 04:26:07,149 synthtool > Executing /tmpfs/src/git/autosynth/working_repo/synth.py.
.eslintignore
.eslintrc.yml
.github/ISSUE_TEMPLATE/bug_report.md
.github/ISSUE_TEMPLATE/feature_request.md
.github/ISSUE_TEMPLATE/support_request.md
.github/PULL_REQUEST_TEMPLATE.md
.github/publish.yml
.github/release-please.yml
.github/workflows/ci.yaml
.kokoro/common.cfg
.kokoro/continuous/node10/common.cfg
.kokoro/continuous/node10/docs.cfg
.kokoro/continuous/node10/lint.cfg
.kokoro/continuous/node10/samples-test.cfg
.kokoro/continuous/node10/system-test.cfg
.kokoro/continuous/node10/test.cfg
.kokoro/continuous/node12/common.cfg
.kokoro/continuous/node12/test.cfg
.kokoro/continuous/node8/common.cfg
.kokoro/continuous/node8/test.cfg
.kokoro/docs.sh
.kokoro/lint.sh
.kokoro/presubmit/node10/common.cfg
.kokoro/presubmit/node10/docs.cfg
.kokoro/presubmit/node10/lint.cfg
.kokoro/presubmit/node10/samples-test.cfg
.kokoro/presubmit/node10/system-test.cfg
.kokoro/presubmit/node10/test.cfg
.kokoro/presubmit/node12/common.cfg
.kokoro/presubmit/node12/test.cfg
.kokoro/presubmit/node8/common.cfg
.kokoro/presubmit/node8/test.cfg
.kokoro/presubmit/windows/common.cfg
.kokoro/presubmit/windows/test.cfg
.kokoro/publish.sh
.kokoro/release/docs.cfg
.kokoro/release/docs.sh
.kokoro/release/publish.cfg
.kokoro/samples-test.sh
.kokoro/system-test.sh
.kokoro/test.bat
.kokoro/test.sh
.kokoro/trampoline.sh
.mocharc.js
.nycrc
.prettierignore
.prettierrc
CODE_OF_CONDUCT.md
CONTRIBUTING.md
LICENSE
README.md
codecov.yaml
renovate.json
samples/README.md
2020-03-22 04:26:07,632 synthtool > Wrote metadata to synth.metadata.

```
</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the Cloud Storage API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can we look into making ACL operations really nice for users?
4 participants