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

Odd differences between signed and unsigned auth credentials #1070

Open
kalepail opened this issue Sep 27, 2024 · 4 comments
Open

Odd differences between signed and unsigned auth credentials #1070

kalepail opened this issue Sep 27, 2024 · 4 comments
Assignees
Labels

Comments

@kalepail
Copy link
Contributor

Describe the bug

What version are you on?
12.3.0

To Reproduce
Review this unsigned auth credential

AAAAAQAAAAF6gVF2E0+pwV5vWTPtG5VyHJ/X54JHeZUczj7CnvD123q5eZAtqGIwAAAAAAAAAAE=

Now review that same credential after it's been signed

AAAAAQAAAAF6gVF2E0+pwV5vWTPtG5VyHJ/X54JHeZUczj7CnvD123q5eZAtqGIwAAJ7YwAAABAAAAABAAAAAQAAABEAAAABAAAAAQAAABAAAAABAAAAAgAAAA8AAAAHRWQyNTUxOQAAAAANAAAAIGsGKxYxF08hS1ap4iEHm23VophJPlpB5uFAHcIbUBDSAAAAEAAAAAEAAAACAAAADwAAAAdFZDI1NTE5AAAAAA0AAABAtUp2f4JU119X1pK1j641RzQO7xJnpE+Qj4NIfCKQwfvLTtAJD4bLiiSR+rZjIjcRjak2TvQua+4zEzK2FqdKBg==

An unsigned credential will mark

entry.credentials().switch() === xdr.SorobanCredentialsType.sorobanCredentialsAddress()

as true
But signed will mark that same check as false

Expected behavior
Signed auth entries should not change their underlying credential type.

Additional context
This issue breaks this check if you're passing in an entry which has already been signed. Thus breaking multisig scenarios for custom accounts https://github.com/stellar/js-stellar-sdk/blob/master/src/contract/assembled_transaction.ts#L839-L842

@kalepail kalepail added the bug label Sep 27, 2024
@github-project-automation github-project-automation bot moved this to Backlog (Not Ready) in DevX Sep 27, 2024
@kalepail
Copy link
Contributor Author

kalepail commented Sep 27, 2024

"Fun" fact if you change the check to

entry.credentials().switch().name === 'sorobanCredentialsAddress'

it will pass. So likely a type issue. Especially given that once you do that further checks down the stack will fail like:

const authEntryAddress = Address.fromScAddress(
  entry.credentials().address().address(),
).toString();

will fail with

Uncaught (in promise) Error: Unsupported address type
    at Function.fromScAddress (@stellar_stellar-sdk_contract.js?v=c0695b00:11771:21)
    at _AssembledTransaction.signAuthEntries (@stellar_stellar-sdk_contract.js?v=c0695b00:23789:59)
    at PasskeyKit.sign (kit.ts:346:19)
    at HTMLButtonElement.policyTransfer (App.svelte:332:17)

@ooochoche
Copy link

ooochoche commented Sep 27, 2024

@kalepail Can I work on this?

@kalepail
Copy link
Contributor Author

The way I'm getting around this is just by cloning the credentials XDR. I don't know what that works but it seems to.
e.g. const credentials = xdr.SorobanCredentials.fromXDR(entry.credentials().toXDR())

chadoh added a commit to AhaLabs/js-stellar-sdk that referenced this issue Sep 27, 2024
Fixes: stellar#1030

Soroban allows contract authors to call `require_auth()` not only on a
G-address (for users), but also on a C-address (for contracts). This
will result in a cross-contract call to the `__check_auth` function in
the signing contract. This enables all sorts of powerful and novel
use-cases. See https://developers.stellar.org/docs/build/guides/conventions/check-auth-tutorials#customaccountinterface-implementation

Just as if one account submits a transaction (meaning that account signs
the transaction envelope) but the contract calls `require_auth` on a
different one, the app author will need to use
`needsNonInvokerSigningBy` and `signAuthEntries`, so app authors benefit
from these functions when `require_auth` is called on a contract.

This fixes various assumptions that these functions made to also work
with this sort of cross-contract auth.

- `needsNonInvokerSigningBy` now returns contract addresses
- `sign` ignores contracts in the `needsNonInvokerSigningBy` list, since
  the actual authorization will happen via cross-contract call
- `signAuthEntry` now takes an `address` instead of a `publicKey`, since
  this address may be a user's public key (a G-address) or a contract
  address. Furthermore, it allows setting a custom `authorizeEntry`, so
  that app and library authors implementing custom cross-contract auth
  can more easily assemble complex transactions.

Additional changes:

- switch to new test cases from AhaLabs/soroban-test-contracts
- switch to `stellar` instead of `soroban`
- use latest cli version (this version fixes a problem that some of the
  tests were working around, so they were removed)
- add (untested) workaround for stellar#1070
@chadoh
Copy link
Contributor

chadoh commented Sep 27, 2024

Added @kalepail's workaround to the implementation of signAuthEntries in #1044

chadoh added a commit to AhaLabs/js-stellar-sdk that referenced this issue Oct 2, 2024
Fixes: stellar#1030

Soroban allows contract authors to call `require_auth()` not only on a
G-address (for users), but also on a C-address (for contracts). This
will result in a cross-contract call to the `__check_auth` function in
the signing contract. This enables all sorts of powerful and novel
use-cases. See https://developers.stellar.org/docs/build/guides/conventions/check-auth-tutorials#customaccountinterface-implementation

Just as if one account submits a transaction (meaning that account signs
the transaction envelope) but the contract calls `require_auth` on a
different one, the app author will need to use
`needsNonInvokerSigningBy` and `signAuthEntries`, so app authors benefit
from these functions when `require_auth` is called on a contract.

This fixes various assumptions that these functions made to also work
with this sort of cross-contract auth.

- `needsNonInvokerSigningBy` now returns contract addresses
- `sign` ignores contracts in the `needsNonInvokerSigningBy` list, since
  the actual authorization will happen via cross-contract call
- `signAuthEntry` now takes an `address` instead of a `publicKey`, since
  this address may be a user's public key (a G-address) or a contract
  address. Furthermore, it allows setting a custom `authorizeEntry`, so
  that app and library authors implementing custom cross-contract auth
  can more easily assemble complex transactions.

Additional changes:

- switch to new test cases from AhaLabs/soroban-test-contracts, embed as
  git submodule
- switch to `stellar` instead of `soroban`
- use latest cli version (this version fixes a problem that some of the
  tests were working around, so they were removed)
- add (untested) workaround for stellar#1070
chadoh added a commit to AhaLabs/js-stellar-sdk that referenced this issue Oct 2, 2024
Fixes: stellar#1030

Soroban allows contract authors to call `require_auth()` not only on a
G-address (for users), but also on a C-address (for contracts). This
will result in a cross-contract call to the `__check_auth` function in
the signing contract. This enables all sorts of powerful and novel
use-cases. See https://developers.stellar.org/docs/build/guides/conventions/check-auth-tutorials#customaccountinterface-implementation

Just as if one account submits a transaction (meaning that account signs
the transaction envelope) but the contract calls `require_auth` on a
different one, the app author will need to use
`needsNonInvokerSigningBy` and `signAuthEntries`, so app authors benefit
from these functions when `require_auth` is called on a contract.

This fixes various assumptions that these functions made to also work
with this sort of cross-contract auth.

- `needsNonInvokerSigningBy` now returns contract addresses
- `sign` ignores contracts in the `needsNonInvokerSigningBy` list, since
  the actual authorization will happen via cross-contract call
- `signAuthEntry` now takes an `address` instead of a `publicKey`, since
  this address may be a user's public key (a G-address) or a contract
  address. Furthermore, it allows setting a custom `authorizeEntry`, so
  that app and library authors implementing custom cross-contract auth
  can more easily assemble complex transactions.

Additional changes:

- switch to new test cases from AhaLabs/soroban-test-contracts, embed as
  git submodule
- switch to `stellar` instead of `soroban`
- use latest cli version (this version fixes a problem that some of the
  tests were working around, so they were removed)
- add (untested) workaround for stellar#1070
chadoh added a commit to AhaLabs/js-stellar-sdk that referenced this issue Oct 2, 2024
Fixes: stellar#1030

Soroban allows contract authors to call `require_auth()` not only on a
G-address (for users), but also on a C-address (for contracts). This
will result in a cross-contract call to the `__check_auth` function in
the signing contract. This enables all sorts of powerful and novel
use-cases. See https://developers.stellar.org/docs/build/guides/conventions/check-auth-tutorials#customaccountinterface-implementation

Just as if one account submits a transaction (meaning that account signs
the transaction envelope) but the contract calls `require_auth` on a
different one, the app author will need to use
`needsNonInvokerSigningBy` and `signAuthEntries`, so app authors benefit
from these functions when `require_auth` is called on a contract.

This fixes various assumptions that these functions made to also work
with this sort of cross-contract auth.

- `needsNonInvokerSigningBy` now returns contract addresses
- `sign` ignores contracts in the `needsNonInvokerSigningBy` list, since
  the actual authorization will happen via cross-contract call
- `signAuthEntry` now takes an `address` instead of a `publicKey`, since
  this address may be a user's public key (a G-address) or a contract
  address. Furthermore, it allows setting a custom `authorizeEntry`, so
  that app and library authors implementing custom cross-contract auth
  can more easily assemble complex transactions.

Additional changes:

- switch to new test cases from AhaLabs/soroban-test-contracts, embed as
  git submodule
- switch to `stellar` instead of `soroban`
- use latest cli version (this version fixes a problem that some of the
  tests were working around, so they were removed)
- add (untested) workaround for stellar#1070
chadoh added a commit to AhaLabs/js-stellar-sdk that referenced this issue Oct 2, 2024
Fixes: stellar#1030

Soroban allows contract authors to call `require_auth()` not only on a
G-address (for users), but also on a C-address (for contracts). This
will result in a cross-contract call to the `__check_auth` function in
the signing contract. This enables all sorts of powerful and novel
use-cases. See https://developers.stellar.org/docs/build/guides/conventions/check-auth-tutorials#customaccountinterface-implementation

Just as if one account submits a transaction (meaning that account signs
the transaction envelope) but the contract calls `require_auth` on a
different one, the app author will need to use
`needsNonInvokerSigningBy` and `signAuthEntries`, so app authors benefit
from these functions when `require_auth` is called on a contract.

This fixes various assumptions that these functions made to also work
with this sort of cross-contract auth.

- `needsNonInvokerSigningBy` now returns contract addresses
- `sign` ignores contracts in the `needsNonInvokerSigningBy` list, since
  the actual authorization will happen via cross-contract call
- `signAuthEntry` now takes an `address` instead of a `publicKey`, since
  this address may be a user's public key (a G-address) or a contract
  address. Furthermore, it allows setting a custom `authorizeEntry`, so
  that app and library authors implementing custom cross-contract auth
  can more easily assemble complex transactions.

Additional changes:

- switch to new test cases from AhaLabs/soroban-test-contracts, embed as
  git submodule
- switch to `stellar` instead of `soroban`
- use latest cli version (this version fixes a problem that some of the
  tests were working around, so they were removed)
- add (untested) workaround for stellar#1070
chadoh added a commit to AhaLabs/js-stellar-sdk that referenced this issue Oct 3, 2024
Fixes: stellar#1030

Soroban allows contract authors to call `require_auth()` not only on a
G-address (for users), but also on a C-address (for contracts). This
will result in a cross-contract call to the `__check_auth` function in
the signing contract. This enables all sorts of powerful and novel
use-cases. See https://developers.stellar.org/docs/build/guides/conventions/check-auth-tutorials#customaccountinterface-implementation

Just as if one account submits a transaction (meaning that account signs
the transaction envelope) but the contract calls `require_auth` on a
different one, the app author will need to use
`needsNonInvokerSigningBy` and `signAuthEntries`, so app authors benefit
from these functions when `require_auth` is called on a contract.

This fixes various assumptions that these functions made to also work
with this sort of cross-contract auth.

- `needsNonInvokerSigningBy` now returns contract addresses
- `sign` ignores contracts in the `needsNonInvokerSigningBy` list, since
  the actual authorization will happen via cross-contract call
- `signAuthEntry` now takes an `address` instead of a `publicKey`, since
  this address may be a user's public key (a G-address) or a contract
  address. Furthermore, it allows setting a custom `authorizeEntry`, so
  that app and library authors implementing custom cross-contract auth
  can more easily assemble complex transactions.

Additional changes:

- switch to new test cases from AhaLabs/soroban-test-contracts, embed as
  git submodule
- switch to `stellar` instead of `soroban`
- use latest cli version (this version fixes a problem that some of the
  tests were working around, so they were removed)
- add (untested) workaround for stellar#1070
chadoh added a commit to AhaLabs/js-stellar-sdk that referenced this issue Oct 3, 2024
Fixes: stellar#1030

Soroban allows contract authors to call `require_auth()` not only on a
G-address (for users), but also on a C-address (for contracts). This
will result in a cross-contract call to the `__check_auth` function in
the signing contract. This enables all sorts of powerful and novel
use-cases. See https://developers.stellar.org/docs/build/guides/conventions/check-auth-tutorials#customaccountinterface-implementation

Just as if one account submits a transaction (meaning that account signs
the transaction envelope) but the contract calls `require_auth` on a
different one, the app author will need to use
`needsNonInvokerSigningBy` and `signAuthEntries`, so app authors benefit
from these functions when `require_auth` is called on a contract.

This fixes various assumptions that these functions made to also work
with this sort of cross-contract auth.

- `needsNonInvokerSigningBy` now returns contract addresses
- `sign` ignores contracts in the `needsNonInvokerSigningBy` list, since
  the actual authorization will happen via cross-contract call
- `signAuthEntry` now takes an `address` instead of a `publicKey`, since
  this address may be a user's public key (a G-address) or a contract
  address. Furthermore, it allows setting a custom `authorizeEntry`, so
  that app and library authors implementing custom cross-contract auth
  can more easily assemble complex transactions.

Additional changes:

- switch to new test cases from AhaLabs/soroban-test-contracts, embed as
  git submodule
- switch to `stellar` instead of `soroban`
- use latest cli version (this version fixes a problem that some of the
  tests were working around, so they were removed)
- add (untested) workaround for stellar#1070
chadoh added a commit to AhaLabs/js-stellar-sdk that referenced this issue Oct 3, 2024
Fixes: stellar#1030

Soroban allows contract authors to call `require_auth()` not only on a
G-address (for users), but also on a C-address (for contracts). This
will result in a cross-contract call to the `__check_auth` function in
the signing contract. This enables all sorts of powerful and novel
use-cases. See https://developers.stellar.org/docs/build/guides/conventions/check-auth-tutorials#customaccountinterface-implementation

Just as if one account submits a transaction (meaning that account signs
the transaction envelope) but the contract calls `require_auth` on a
different one, the app author will need to use
`needsNonInvokerSigningBy` and `signAuthEntries`, so app authors benefit
from these functions when `require_auth` is called on a contract.

This fixes various assumptions that these functions made to also work
with this sort of cross-contract auth.

- `needsNonInvokerSigningBy` now returns contract addresses
- `sign` ignores contracts in the `needsNonInvokerSigningBy` list, since
  the actual authorization will happen via cross-contract call
- `signAuthEntry` now takes an `address` instead of a `publicKey`, since
  this address may be a user's public key (a G-address) or a contract
  address. Furthermore, it allows setting a custom `authorizeEntry`, so
  that app and library authors implementing custom cross-contract auth
  can more easily assemble complex transactions.

Additional changes:

- switch to new test cases from AhaLabs/soroban-test-contracts, embed as
  git submodule
- switch to `stellar` instead of `soroban`
- use latest cli version (this version fixes a problem that some of the
  tests were working around, so they were removed)
- add (untested) workaround for stellar#1070
kalepail pushed a commit that referenced this issue Oct 3, 2024
Fixes: #1030

Soroban allows contract authors to call `require_auth()` not only on a
G-address (for users), but also on a C-address (for contracts). This
will result in a cross-contract call to the `__check_auth` function in
the signing contract. This enables all sorts of powerful and novel
use-cases. See https://developers.stellar.org/docs/build/guides/conventions/check-auth-tutorials#customaccountinterface-implementation

Just as if one account submits a transaction (meaning that account signs
the transaction envelope) but the contract calls `require_auth` on a
different one, the app author will need to use
`needsNonInvokerSigningBy` and `signAuthEntries`, so app authors benefit
from these functions when `require_auth` is called on a contract.

This fixes various assumptions that these functions made to also work
with this sort of cross-contract auth.

- `needsNonInvokerSigningBy` now returns contract addresses
- `sign` ignores contracts in the `needsNonInvokerSigningBy` list, since
  the actual authorization will happen via cross-contract call
- `signAuthEntry` now takes an `address` instead of a `publicKey`, since
  this address may be a user's public key (a G-address) or a contract
  address. Furthermore, it allows setting a custom `authorizeEntry`, so
  that app and library authors implementing custom cross-contract auth
  can more easily assemble complex transactions.

Additional changes:

- switch to new test cases from AhaLabs/soroban-test-contracts, embed as
  git submodule
- switch to `stellar` instead of `soroban`
- use latest cli version (this version fixes a problem that some of the
  tests were working around, so they were removed)
- add (untested) workaround for #1070
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Backlog (Not Ready)
Development

No branches or pull requests

3 participants