Implementing the Firebase Security Rules API#604
Conversation
| * @returns {Promise<T>} A promise that fulfills with the resource. | ||
| */ | ||
| public getResource<T>(name: string): Promise<T> { | ||
| if (!name.startsWith('projects/')) { |
There was a problem hiding this comment.
may be worth regexing this to catch malformed project ids (which, for instance, I think need to be lowercase).
There was a problem hiding this comment.
I simplified things a bit here by pushing the projectId to the constructor of the RulesApiClient. Now it stores a partial URL as part of its state:
this.url = `${RULES_API_URL}/projects/${projectId}`;
Callers should pass in the resource names without project ID:
client.getResource('rulesets/my-ruleset');
As for project ID validation, we are doing the same check here as our other modules (Auth, FCM), and so it should be sufficient.
There was a problem hiding this comment.
I'll defer to you here, but:
- there aren't that many kinds of resources
- it may be easier to explicitly spell them all out as unique methods:
client.getRuleset(<ruleset_name>)
client.listRulesets()
client.getRelease()
client.listReleases()
which directly encodes the exposed backend API. Then add another layer on top of that.
There was a problem hiding this comment.
This is a good idea. Will implement in the next PRs.
|
Thanks @ryanpbrewster. I've made a pass and addressed your comments. |
ryanpbrewster
left a comment
ryanpbrewster
left a comment
There was a problem hiding this comment.
LGTM as-is, if you opt to refactor to match the backend API feel free to re-assign me (alternately: feel free to do this in a separate PR)
rachelmyers
left a comment
rachelmyers
left a comment
There was a problem hiding this comment.
This looks good. There were a few interfaces that I didn't expect, and left a comment in-line for those.
🚢
| readonly source: { | ||
| readonly files: RulesFile[]; | ||
| }; | ||
| } |
There was a problem hiding this comment.
Are Release and RulesetResponse additions to what we proposed?
There was a problem hiding this comment.
These are part of the internal implementation. They are not exported and also won't be added to the typings.
src/security-rules/security-rules.ts
Outdated
| } | ||
|
|
||
| /** | ||
| * Representa a set of Firebase security rules. |
* Implementing the Firebase Security Rules API (#604) * Implementing the Firebase Security Rules API * More argument validation and assertions * Cleaning up the rules impl * Internal API renamed * Fixing a typo in a comment * Implemented createRulesFileFromSource() and createRuleset() APIs (#607) * Implementing the Firebase Security Rules API * More argument validation and assertions * Adding the rest of the CRUD operations for rulesets * Cleaning up the rules impl * Cleaned up tests * Adding some missing comments * Removing support for multiple rules files in create() * Implemented the deleteRuleset() API (#609) * Added deleteRuleset API * Merged with source * Implemented the API for releasing rulesets (#610) * Implemented the API for releasing rulesets * Removed createRelease logic * Updated comment * Added the getStorageRuleset() API (#613) * Implemented the API for releasing rulesets * Removed createRelease logic * Added getStorageRules() API * Removed some redundant tests * Implementing the remaining releaseRuleset APIs (#616) * Implemented the listRulesetMetadata() API (#622) * Adding the rules API to the public API surface (#625) * Added rules API to the public admin namespace * Updated docs * Addressing comments regarding the d.ts file * Updated App typings * Rules integration tests (#633) * Rules integration tests * Refactored by adding some helper methods * Cleaned up some conditionals * Added verification for deleteRuleset test * Renamed tempRulesets * Handling ruleset limit exceeded error (#636) * Fixing alignment of an annotation * Updated comments
3 participants
Added the base framework for the new security rules API (nothing new exposed in the public API --i.e.
adminnamespace -- yet). This PR specifically adds theSecurityRulesclass and the following 2 methods:getRuleset()getFirestoreRuleset()Also defined the types
Ruleset,RulesetMetadataandRulesFilewhich will be added to the public API in the future.go/firebase-rules-admin