Skip to content

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

Merged
ifropc merged 12 commits intomasterfrom
partner-275-auth-header
Mar 28, 2024
Merged

[PARTNER-275] Add Authorization header for SEP-10 GET /Auth#1470
ifropc merged 12 commits intomasterfrom
partner-275-auth-header

Conversation

@ifropc
Copy link
Copy Markdown

@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.

@ifropc ifropc force-pushed the partner-275-auth-header branch from eaa9487 to 4f37f5a Compare March 20, 2024 23:47
@ifropc ifropc requested review from CassioMG and JakeUrban March 20, 2024 23:47
Copy link
Copy Markdown
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.

Comment thread ecosystem/sep-0010.md Outdated
Comment thread ecosystem/sep-0010.md
Comment thread ecosystem/sep-0010.md Outdated
Copy link
Copy Markdown
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

Comment thread ecosystem/sep-0010.md Outdated
Comment thread ecosystem/sep-0010.md
Comment thread ecosystem/sep-0010.md Outdated
Comment thread ecosystem/sep-0010.md Outdated
Comment thread ecosystem/sep-0010.md Outdated
Comment thread ecosystem/sep-0010.md Outdated
Copy link
Copy Markdown
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
Comment thread ecosystem/sep-0010.md Outdated
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
Copy Markdown
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
@ifropc ifropc deleted the partner-275-auth-header branch March 28, 2024 21:16
ifropc referenced this pull request in stellar/anchor-platform Apr 15, 2024
…1303)

### Description

- Implementation of the protocol change
[https://github.com/stellar/stellar-protocol/pull/1470](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
@stellar stellar deleted a comment from gre3n3yes34 Oct 28, 2024
@stellar stellar deleted a comment from gre3n3yes34 Oct 28, 2024
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