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

[PARTNER-275] Add Authorization header for SEP-10 GET /Auth #1470

Merged
merged 12 commits into from
Mar 28, 2024

Conversation

Ifropc
Copy link
Contributor

@Ifropc Ifropc commented Mar 20, 2024

In the protocol change, an optional Authorization header was added for GET <WEB_AUTH_ENDPOINT> endpoint. The header should contain a signed JWT token (using ed25519) with an appropriate key from the request.
For custodial applications, this is a primary Application key, provided in account field.
For non-custodial, this will be the SIGNING_KEY from toml file hosted in the client_domain

The server will validate that the signature is correct, and that URL in the JWT corresponds to the request. It can optionally filter out requests from all clients that are not allowed by the server.

Copy link
Contributor

@JakeUrban JakeUrban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like its 90% there! I added some clarifying questions. Also don't forget to update the version and updated-at field.

ecosystem/sep-0010.md Outdated Show resolved Hide resolved
ecosystem/sep-0010.md Outdated Show resolved Hide resolved
ecosystem/sep-0010.md Outdated Show resolved Hide resolved
Copy link
Contributor

@JakeUrban JakeUrban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the repeated delay, a couple more questions / ideas

ecosystem/sep-0010.md Outdated Show resolved Hide resolved
ecosystem/sep-0010.md Show resolved Hide resolved
ecosystem/sep-0010.md Outdated Show resolved Hide resolved
ecosystem/sep-0010.md Outdated Show resolved Hide resolved
ecosystem/sep-0010.md Outdated Show resolved Hide resolved
ecosystem/sep-0010.md Outdated Show resolved Hide resolved
Copy link
Contributor

@CassioMG CassioMG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

JakeUrban
JakeUrban previously approved these changes Mar 28, 2024
Client must then correctly sign the payload with appropriate Stellar private key. To choose the private key client
application should follow this steps:

- If `client_domain` is specified, the token must be signed with the _Client Domain Account_ (i.e. [SEP-1] defined
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- If `client_domain` is specified, the token must be signed with the _Client Domain Account_ (i.e. [SEP-1] defined
- If `client_domain` is specified, the token must be signed with the **Client Domain Account** (i.e. [SEP-1] defined

@Ifropc Ifropc merged commit ad012e2 into master Mar 28, 2024
3 checks passed
@Ifropc Ifropc deleted the partner-275-auth-header branch March 28, 2024 21:16
Ifropc added a commit to stellar/java-stellar-anchor-sdk that referenced this pull request Apr 15, 2024
…1303)

### Description

- Implementation of the protocol change
[stellar/stellar-protocol#1470: Add
Authorization header for SEP-10 GET /Auth)
- Migration to the latest version of JJWT library
- Improvement of working with secrets in the code
- Improved JWT secret validation

### Context

- Protocol change
- Old JJWT library doesn't have a good support for the algorithm used by
the protocol change
- Refactored how we work with JWT secrets in the code: use SecretKey
instead of Sting
- Previously, weak keys were not validated properly and was allowed to
use, even though it's against the guidelines. Now, this keys are
properly validated using underlying JJWT library.

### Testing

- `./gradlew test`
- Add tests for SEP-10 auth header 

### Documentation

N/A

### Known limitations

N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants