feat: add support for SEP-10 v3.1.0. #319
Conversation
There was a problem hiding this comment.
this looks good to me but I'll yield to @leighmcculloch for the final approval
There was a problem hiding this comment.
Looks good to me, although one concern. @JakeUrban could you chime in?
| * @return {@link ChallengeTransaction}, the decoded transaction envelope and client account ID contained within. | ||
| * @throws InvalidSep10ChallengeException If the SEP-0010 validation fails, the exception will be thrown. | ||
| * @throws IOException If read XDR string fails, the exception will be thrown. | ||
| */ | ||
| public static ChallengeTransaction readChallengeTransaction(String challengeXdr, String serverAccountId, Network network, String[] domainNames) throws InvalidSep10ChallengeException, IOException { | ||
| public static ChallengeTransaction readChallengeTransaction(String challengeXdr, String serverAccountId, Network network, String[] domainNames, String webAuthDomain) throws InvalidSep10ChallengeException, IOException { |
There was a problem hiding this comment.
In the Go SDK we need to put the webauth domain before the home domains to preserve an odd but awkward case that can break applications without them realizing, so I think there would be value in having all SDKs have that field first for consistency. But it also doesn't really matter. cc @JakeUrban
There was a problem hiding this comment.
I don't think it makes sense to try to keep function signatures consistent across languages. I'm pretty sure that ship has already sailed anyway. The change made looks good to me 👍
| * @return {@link ChallengeTransaction}, the decoded transaction envelope and client account ID contained within. | ||
| * @throws InvalidSep10ChallengeException If the SEP-0010 validation fails, the exception will be thrown. | ||
| * @throws IOException If read XDR string fails, the exception will be thrown. | ||
| */ | ||
| public static ChallengeTransaction readChallengeTransaction(String challengeXdr, String serverAccountId, Network network, String[] domainNames) throws InvalidSep10ChallengeException, IOException { | ||
| public static ChallengeTransaction readChallengeTransaction(String challengeXdr, String serverAccountId, Network network, String[] domainNames, String webAuthDomain) throws InvalidSep10ChallengeException, IOException { |
There was a problem hiding this comment.
I don't think it makes sense to try to keep function signatures consistent across languages. I'm pretty sure that ship has already sailed anyway. The change made looks good to me 👍
|
@overcat could you try rebasing your branch on top of master? we need to retrigger circle ci so that the PR can be merged |
overcat
force-pushed
the
dev-sep10-v3.1.0
branch
from
e697d86
to
38a11da
Compare
|
Thank you all. |
resolve #315
webAuthDomainparameter was added to the following functionsSep10Challenge#newChallenge(KeyPair, Network, String, String, String, TimeBounds)Sep10Challenge#readChallengeTransaction(String, String, Network, String, String)Sep10Challenge#readChallengeTransaction(String, String, Network, String[], String)Sep10Challenge#verifyChallengeTransactionSigners(String, String, Network, String, String, Set)Sep10Challenge#verifyChallengeTransactionSigners(String, String, Network, String[], String, Set)Sep10Challenge#verifyChallengeTransactionThreshold(String, String, Network, String[], String, int, Set)Sep10Challenge#verifyChallengeTransactionThreshold(String, String, Network, String, String, int, Set)webAuthDomainparameter is expected to match the value of the Manage Data operation with the 'web_auth_domain' key, if present.