Merge pull request #279 from onflow/josh/entitlements-guidance

Update guidance for capabilities and entitlements

Author: briandoyle81

docs/anti-patterns.md

@@ -83,9 +83,12 @@ Accidentally exposed fields can be a security hole.
 ### Solution
 
 When writing your smart contract, look at every field and function and make sure
-that require access through an [entitlement](./language/access-control.md#entitlements) (`access(E)`),
+they require access through an [entitlement](./language/access-control.md#entitlements) (`access(E)`),
 or use a non-public [access modifier](./language/access-control.md) like `access(self)`, `access(contract)`, or `access(account)`,
-unless otherwise needed.
+unless you are making a deliberate design decision to allow completely open and unrestricted access to read that field or call that function.
+
+The only functions that should be `access(all)` are `view` functions and the only fields that can be `access(all)` are basic types like numbers or addresses.
+Complex fields like arrays, dictionaries, structs, resources, or capabilities should always be `access(self)`.
 
 ## Capability-Typed public fields are a security hole
 

docs/design-patterns.md

@@ -501,23 +501,36 @@ transaction(provider: Address, name: String) {
 }
 ```
 
-## Check for existing capability before publishing new one
+## Check for existing capabilities before issuing or publishing new ones
 
 ### Problem
 
-When publishing a capability, a capability might be already be published at the specified path.
+When issuing or publishing a capability, a capability might be already be issued or published at the specified path for the desired capability type.
 
 ### Solution
 
-Check if a capability is already published at the given path.
+Check if a capability is already issued and/or published at the given paths.
 
 ### Example
 
 ```cadence
 transaction {
     prepare(signer: auth(Capabilities) &Account) {
-        let capability = signer.capabilities.storage
-            .issue<&ExampleToken.Vault>(/storage/exampleTokenVault)
+        var capability: Capability<&ExampleToken.Vault>? = nil
+
+        // get the capability to the vault at the given storage path if it exists
+        let vaultCaps = account.capabilities.storage.getControllers(forPath: /storage/exampleTokenVault)
+        for cap in vaultCaps {
+            if let cap = cap as? Capability<&ExampleToken.Vault> {
+                capability = cap
+                break
+            }
+        }
+
+        if capability == nil {
+            // issue a new capability to the vault since it wasn't found
+            capability = account.capabilities.storage.issue<&ExampleToken.Vault>(/storage/exampleTokenVault)
+        }
 
         let publicPath = /public/exampleTokenReceiver
 

docs/language/access-control.md

@@ -12,18 +12,20 @@ In Cadence, access control is used in two ways:
    A user is not able to access an object unless they own the object or have a reference to that object. This means that nothing is truly public by default.
 
    Other accounts cannot read or write the objects in an account unless the owner of the account has granted them access by providing references to the objects.
+
+    This kind of access control is covered in [capabilities] and [capability management].
+
+2. Access control within contracts and objects, using access modifiers (`access` keyword).
+
+This page covers the second part of access control, using access modifiers.
 
 :::warning
 
-Remember that in this case, `private` refers to programmatic access to the data with a script or transaction.  It is **not safe** to store secret or private information in a user's account.  The raw data is still public and could be decoded.
+Remember that in this case, `private` refers to programmatic access to the data with a script or transaction.  It is **not safe** to store secret or private information in a user's account.  The raw data is still public and could be decoded by reading the public blockchain data directly.
 
 :::
-   
-   This kind of access control is covered in [capabilities] and [capability management].
-
-1. Access control within contracts and objects, using access modifiers (`access` keyword).
 
-This page covers the second part of access control, using access modifiers.
+## The `access` keyword
 
 All declarations, such as [functions], [composite types], and fields, must be prefixed with an access modifier using the `access` keyword.
 
@@ -36,6 +38,14 @@ access(all)
 fun test() {}
 ```
 
+:::danger
+
+If you prefix a function with `access(all)`, you are likely granting complete and open access for **anyone using any account to call that function.** It is critical that you properly use [entitlements] to restrict sensitive functions to the accounts that need access.
+
+For example, if you create a vault for your users and give the `withdraw` function `access(all)`, anyone can drain that vault if they know where to find it.
+
+:::
+
 ## Types of access control
 
 There are five levels of access control:
@@ -46,9 +56,11 @@ There are five levels of access control:
 
    For example, a public field in a type can be accessed on an instance of the type in an outer scope.
 
-- **Entitled access** — the declaration is only accessible/visible to the owner of the object, or to [references] that are authorized to the required [entitlements].
+- **Entitled access** — the declaration is only accessible/visible to the owner/holder of the object, or to [references] that are authorized to the required [entitlements].
 
    A declaration is made accessible through entitlements by using the `access(E)` syntax, where `E` is a set of one or more entitlements, or a single [entitlement mapping].
+
+   An entitled field acts like an `access(all)` field ONLY if the caller actually holds the concrete resource object and not just a reference to it. In that case, an authorized reference is needed.
 
    A reference is considered authorized to an entitlement if that entitlement appears in the `auth` portion of the reference type.
 

docs/security-best-practices.md

@@ -8,11 +8,148 @@ This is an opinionated list of best practices that Cadence developers should fol
 
 Some practices listed below might overlap with advice in the [Cadence Anti-Patterns] article, which is a recommended read as well.
 
+## Access Control
+
+Do not use the `access(all)` modifier on fields and functions unless absolutely necessary. Prefer `access(self)`, `access(contract)`, `access(account)`, or `access(SomeEntitlement)`. Unintentionally declaring fields or functions as `access(all)` can expose vulnerabilities in your code.
+
+When writing definitions for contracts, structs, or resources, start by declaring all your fields and functions as `access(self)`. If there is a function that needs to be accessible by external code, only declare it as `access(all)` if it is a `view` function:
+
+```cadence
+/// Simplified Bank Account implementation
+access(all) resource BankAccount {
+
+    /// Fields should default to access(self) to be safe
+    /// and be readable through view functions
+    access(self) var balance: UFix64
+
+    /// It is okay to make this function access(all) because it is a view function
+    /// and all blockchain data is public
+    access(all) view fun getBalance(): UFix64 {
+        return self.balance
+    }
+}
+```
+
+If there are any functions that modify state that also need to be callable from external code, use [entitlements] for the access modifiers for those functions:
+
+```cadence
+/// Simplified Vault implementation
+/// Simplified Bank Account implementation
+access(all) resource BankAccount {
+
+    /// Declare Entitlements for state-modifying functions
+    access(all) entitlement Owner
+    access(all) entitlement Depositor
+
+    /// Fields should default to access(self) just to be safe
+    access(self) var balance: UFix64
+
+    /// All non-view functions should be something other than access(all),
+    
+    /// This is only callable by other functions in the type, so it is `access(self)`
+    access(self) fun updateBalance(_ new: UFix64) {
+        self.balance = new
+    }
+
+    /// This function is external, but should only be called by the owner
+    /// so we use the `Owner` entitlement
+    access(Owner) fun withdrawFromAccount(_ amount: UFix64): @BankAccount {
+        self.updateBalance(self.balance - amount)
+        return <-create BankAccount(balance: amount)
+    }
+
+    /// This is also state-modifying, so it should also be restricted with entitlements
+    /// In this case, we can use two entitlements to be more specific
+    /// about who can access (Owner OR Depositor)
+    access(Owner | Depositor) fun depositToAccount(_ from: @BankAccount) {
+        self.updateBalance(self.balance + from.getBalance())
+        destroy from
+    }
+}
+```
+
+## Access Control for Composite-typed Fields
+
+Declaring a field as [`access(all)`] only protects from replacing the field's value, but the value itself can still be mutated if it is mutable. Remember that containers, like dictionaries and arrays, are mutable and composite fields like structs and resources are still mutable through their own functions.
+
+:::danger
+
+This means that if you ever have a field that is a resource, struct, or capability, it should ALWAYS be `access(self)`! If it is `access(all)`, anyone could access it and call its functions, which could be a major vulnerability.
+
+You can still allow external code to access that field, but only through functions that you have defined with `access(SomeEntitlement)`. This way, you can explicitly define how external code can access these fields.
+
+:::
+
+# Capabilities
+
+## Issuing Capabilities
+
+Don't issue and publish capabilities unless absolutely necessary. Anyone can access capabilities that are published. If public access is needed, follow the [principle of least privilege/authority]: make sure that the capability type only grants access to the fields and functions that should be exposed, and nothing else. Ideally, create a capability with a reference type that is unauthorized.
+
+When issuing a capability, a capability of the same type might already be present. It is a good practice to check if a capability already exists with `getControllers()` before creating it. If it already exists, you can reuse it instead of issuing a new one. This prevents you from overloading your account storage and overpaying because of redundant capabilities.
+
+```cadence
+    // Capability to find or issue
+    var flowTokenVaultCap: Capability<auth(FungibleToken.Withdraw) &FlowToken.Vault>? = nil
+
+    // Get all the capabilities that have already been issued for the desired storage path
+    let flowTokenVaultCaps = account.capabilities.storage.getControllers(forPath: /storage/flowTokenVault)
+
+    // Iterate through them to see if there is already one of the needed type
+    for cap in flowTokenVaultCaps {
+        if let cap = cap as? Capability<auth(FungibleToken.Withdraw) &FlowToken.Vault> {
+            flowTokenVaultCap = cap
+            break
+        }
+    }
+
+    // If no capabilities of the needed type are already present,
+    // issue a new one
+    if flowTokenVaultCap == nil {
+        // issue a new entitled capability to the flow token vault
+        flowTokenVaultCap = account.capabilities.storage.issue<auth(FungibleToken.Withdraw) &FlowToken.Vault>(/storage/flowTokenVault)
+    }
+```
+
+## Publishing Capabilities
+
+When publishing a capability, a published capability might already be present. It is a good practice to check if a capability already exists with `borrow` before creating it. This function will return `nil` if the capability does not exist.
+
+```cadence
+// Check if the published capability already exists
+if account.capabilities.borrow<&FlowToken.Vault>(/public/flowTokenReceiver) == nil {
+    // since it doesn't exist yet, we should publish a new one that we created earlier
+    signer.capabilities.publish(
+        receiverCapability,
+        at: /public/flowTokenReceiver
+    )
+}
+```
+
+## Checking Capabilities
+
+If it is necessary to handle the case where borrowing a capability might fail, the `account.check` function can be used to verify that the target exists and has a valid type:
+
+```cadence
+// check if the capability is valid
+if capability.check() {
+    let reference = capability.borrow()
+} else {
+    // do something else if the capability isn't valid
+}
+```
+
+## Capability Access
+
+Ensure capabilities cannot be accessed by unauthorized parties. For example, capabilities should not be accessible through a public field, including public dictionaries or arrays. Exposing a capability in such a way allows anyone to borrow it and to perform all actions that the capability allows, including `access(all)` fields and functions that aren't even in the restricted type of the capability.
+
 ## References
 
 [References] are ephemeral values and cannot be stored. If persistence is required, store a capability and borrow it when needed.
 
-When exposing functionality, provide the least access necessary. When creating an authorized reference, create it with only the minimal set of entitlements required to achieve the desired functionality.
+When exposing functionality in an account, struct, or resource, provide the least access necessary. When creating an authorized reference with [entitlements], create it with only the minimal set of [entitlements] required to achieve the desired functionality.
+
+# Accounts
 
 ## Account storage
 
@@ -28,18 +165,6 @@ Therefore, avoid passing an entitled account reference to a function, and when d
 
 It is preferable to use capabilities over direct account storage access when exposing account data. Using capabilities allows the revocation of access and limits the access to a single value with a certain set of functionality.
 
-## Capabilities
-
-Don't issue and publish capabilities unless really necessary. Anyone can access capabilities that are published. If public access is needed, follow the [principle of least privilege/authority]: make sure that the capability type only grants access to the fields and functions that should be exposed, and nothing else. Ideally, create a capability with a reference type that is unauthorized.
-
-If an entitlement is necessary to access the field or function, ensure it is only used for the particular field or function, and not also by other fields and functions. If needed, introduce a new, fine-grained entitlement.
-
-When publishing a capability, a capability might already be present. It is a good practice to check if a capability already exists with `get` before creating it. This function will return `nil` if the capability does not exist.
-
-If it is necessary to handle the case where borrowing a capability might fail, the `account.check` function can be used to verify that the target exists and has a valid type.
-
-Ensure capabilities cannot be accessed by unauthorized parties. For example, capabilities should not be accessible through a public field, including public dictionaries or arrays. Exposing a capability in such a way allows anyone to borrow it and to perform all actions that the capability allows.
-
 ## Transactions
 
 Audits of Cadence code should also include [transactions], as they may contain arbitrary code, just like in contracts. In addition, they are given full access to the accounts of the transaction's signers (i.e., the transaction is allowed to manipulate the signer's account storage, contracts, and keys).
@@ -58,14 +183,6 @@ Use [intersection types and interfaces]. Always use the most specific type possi
 
 If given a less-specific type, cast to the more specific type that is expected. For example, when implementing the fungible token standard, a user may deposit any fungible token, so the implementation should cast to the expected concrete fungible token type.
 
-## Access control
-
-Declaring a field as [`access(all)`] only protects from replacing the field's value, but the value itself can still be mutated if it is mutable. Remember that containers, like dictionaries and arrays, are mutable.
-
-Prefer non-public access to a mutable state. That state may also be nested. For example, a child may still be mutated even if its parent exposes it through a field with non-settable access.
-
-Do not use the `access(all)` modifier on fields unless necessary. Prefer `access(self)`, or `access(contract)` and `access(account)`, when other types in the contract or account need to have access, and use entitlement-based access for other cases.
-
 <!-- Relative links. Will not render on the page -->
 
 [Cadence Anti-Patterns]: ./design-patterns.md
@@ -76,4 +193,5 @@ Do not use the `access(all)` modifier on fields unless necessary. Prefer `access
 [transactions]: ./language/transactions.md
 [principle of least privilege/authority]: https://en.wikipedia.org/wiki/Principle_of_least_privilege
 [intersection types and interfaces]: ./language/types-and-type-system/intersection-types.md
-[`access(all)`]: ./language/access-control.md
+[`access(all)`]: ./language/access-control.md
+[entitlements]: ./language/access-control.md

docs/tutorial/03-resources.md

@@ -179,7 +179,7 @@ We've already imported the `HelloResource` contract for you and stubbed out a `t
 
 To prepare:
 
-1. Create a `prepare` phase with the `SaveValue` authorization [entitlement] to the user's account.
+1. Create a `prepare` phase with the `SaveValue` authorization [entitlement] to the user's account. This authorizes the transaction to save values or objects anywhere in account storage.  You'll learn more about entitlements in the next lesson.
 1. Use `create` to create a new instance of the `HelloAsset`.
 1. Save the new resource in the user's account.
 1. Inside the `transaction`, stub out the `prepare` phase with the authorization [entitlement]:
@@ -213,7 +213,7 @@ Paths in the storage domain have type `StoragePath`, and paths in the public dom
 
 Paths are **not** strings and do **not** have quotes around them.
 
-Use the account reference with the `SaveValue` authorization [entitlement] to move the new resource into storage located in `/storage/HelloAssetTutorial`:
+Next, use the account reference with the `SaveValue` authorization [entitlement] to move the new resource into storage located in `/storage/HelloAssetTutorial`:
 
 ```cadence
 acct.storage.save(<-newHello, to: /storage/HelloAssetTutorial)
@@ -364,6 +364,7 @@ In real applications, you need to check the location path you are storing in to
        // Existing code...
    }
    ```
+   This [entitlement] makes it so you can borrow a reference to a value in the account's storage in addition to being able to save a value.
 1. Add a `transaction`-level (similar to contract-level or class-level) variable to store a result `String`.
 
    - Similar to a class-level variable in other languages, these go at the top, inside the `transaction` scope, but not inside anything else. They are accessible in both the `prepare` and `execute` statements of a transaction: