StandardScrypt has incorrect key for MemoryCost when creating http request

[VictorKeil]

trafficstars

Describe your environment

• Operating System version: macOS Monterey V12.3
• Firebase SDK version: 4
• Library version: 4.7.1
• Firebase Product: auth

Describe the problem

ImportUsers request failed with INVALID_HASH_PARAMETERS. After digging through the code and comparing with the Node SDK I noticed that the key for hash.StandardScrypt.MemoryCost was different between the two. When marshaling the hash config into an http request, the correct key for that field is "cpuMemCost", as opposed to "memoryCost", which is the current value.

Steps to reproduce:

Make an ImportUsers request with hash.StandardScrypt hash config as an option.

Relevant Code:

var client *auth.Client
// Initialize client

config := hash.StandardScrypt{
	MemoryCost:       1024,
	Parallelization:  16,
	BlockSize:        8,
	DerivedKeyLength: 64,
}

_, err := client.ImportUsers(ctx, users, auth.WithHash(config))

Firebase response:

{
  "error": {
    "code": 400,
    "message": "INVALID_HASH_PARAMETER",
    "errors": [
      {
        "message": "INVALID_HASH_PARAMETER",
        "domain": "global",
        "reason": "invalid"
      }
    ]
  }
}

Fix:

Just change the key value returned from hash.StandardScrypt.Config() to "cpuMemCost". Should I just do this with a PR? Your guidelines said to post an issue first. Thanks!

[lahirumaramba]

Hi @VictorKeil Thank you filing this issue. You are right! It looks like cpuMemCost is used for STANDARD_SCRYPT hashing function.

Hi @prameshj, according to the REST API it looks like memoryCost is used for SCRYPT hashing function and cpuMemCost is used for STANDARD_SCRYPT. I checked the Node.js SDK and it seems like we only expose memoryCost field and copies the value over to cpuMemCost in the implementation if STANDARD_SCRYPT is used [ref]. Do you think it makes sense to do something similar in the Go SDK?

[prameshj]

For this specific issue, I think we just need to modify this line from"memoryCost" to "cpuMemoryCost" as Victor pointed out.

~~memoryCost isn't exposed, from what I can tell. It is hardcoded to 1024. If we do expose it, then makes sense to expose a single parameter and copy it to the correct API field internally, like in node.js.~~ memoryCost is indeed exposed, I looked at the snippets in https://github.com/firebase/firebase-admin-go/blob/bb055ed1cfbe6224367c63caedc4ba72f1437dcd/snippets/auth.go#L519 and incorrectly mentioned that the field is not exposed.

I think it makes sense to have the single exposed field and internally write them to the specific api request field.

[prameshj]

/assign @VictorKeil

[lahirumaramba]

This should be now fixed in #508 Thank you.