feat: auth by refresh and session tokens.

[youngsofun]

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

add new endpoint /v1/session/login and /v1/session/renew.

/v1/session/login return refresh_token and session_token
session_token is used to auth /v1/query, TTL = 1h
refresh_token is used to auth /v1/session/renew, TTL=4h

token is verified by store its hash as key in meta, and is cached in memory.

todo（small enhancement）:

• make nonce safer
• check session_id or nonce in addition to hash exists

Tests

• Unit Test
• Logic Test
• Benchmark Test
• No Test - Explain why

Type of change

• Bug Fix (non-breaking change which fixes an issue)
• New Feature (non-breaking change which adds functionality)
• Breaking Change (fix or feature that could cause existing functionality not to work as expected)
• Documentation Update
• Refactoring
• Performance Improvement
• Other (please describe):

---

This change is 

[drmingdrmer]

If reusing CrudMgr is possible, it would greatly simplify things.

I haven't reviewed all the changes yet. I'm waiting for confirmation on the solution above.

Reviewed 17 of 38 files at r1.
Reviewable status: 16 of 38 files reviewed, 5 unresolved discussions (waiting on @youngsofun)

---

src/meta/app/src/principal/user_token.rs line 24 at r1 (raw file):

    Refresh = 1,
    Session = 2,
}

What is Refresh token? It sounds like an action to refresh a token. What's the difference between theses two kind of token? Please add some doc to explain their purpose and usage scenario.

Code quote:

pub enum TokenType {
    Refresh = 1,
    Session = 2,
}

---

src/meta/app/src/principal/user_token.rs line 30 at r1 (raw file):

    pub token_type: TokenType,
    // used to delete refresh token when close session
    pub parent: Option<String>,

Suggestion:

    /// used to delete refresh token when close session
    pub parent: Option<String>,

---

src/query/management/src/token/token_api.rs line 19 at r1 (raw file):

#[async_trait::async_trait]
pub trait TokenApi: Sync + Send {

If possible, re-use CrudMgr to implement a simple crud manager, such as ConnectionMgr
https://github.com/datafuselabs/databend/blob/98652d1a0fca41e8982b217362d9d1339300c86d/src/query/management/src/connection.rs#L18

---

src/query/management/src/token/token_api.rs line 30 at r1 (raw file):

    async fn get_token(&self, token_hash: &str) -> Result<Option<QueryTokenInfo>>;

    async fn drop_token(&self, token_hash: &str) -> Result<()>;

Add doc comment to the trait method explaining their purposes and expected behavior of the implementation.

Code quote:

    async fn upsert_token(
        &self,
        token_hash: &str,
        token_info: QueryTokenInfo,
        ttl_in_secs: u64,
        is_update: bool,
    ) -> Result<bool>;

    async fn get_token(&self, token_hash: &str) -> Result<Option<QueryTokenInfo>>;

    async fn drop_token(&self, token_hash: &str) -> Result<()>;

---

src/query/management/src/token/token_mgr.rs line 64 at r1 (raw file):

    ) -> Result<bool> {
        let ident = self.token_ident(token_hash);
        let seq = MatchSeq::GE(if is_update { 1 } else { 0 });

is_update here is not very clear about its purpose:
replace_existing would be better reflecting its behavior.

Avoid using plain number duration. Use Duration type for ttl.

Code quote:

        is_update: bool,
    ) -> Result<bool> {
        let ident = self.token_ident(token_hash);
        let seq = MatchSeq::GE(if is_update { 1 } else { 0 });

[youngsofun]

If reusing CrudMgr is possible, it would greatly simplify things.

thanks， I wiil try it

---

seem we can not use it because TTL is important here.
btw, is it ok to upsert a key with a longer or shorter TTL?

I will add some docs soon.

[youngsofun]

Add doc comment to the trait method explaining their purposes and expected behavior of the implementation.

I will remove the trait.

[youngsofun]

@drmingdrmer plz review again

[drmingdrmer]

Reviewed 19 of 38 files at r1, 2 of 2 files at r2, 3 of 3 files at r3, 6 of 6 files at r4, all commit messages.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @flaneur2020, @hantmac, @sundy-li, and @youngsofun)

---

src/meta/app/src/principal/user_token.rs line 30 at r1 (raw file):

    pub token_type: TokenType,
    // used to delete refresh token when close session
    pub parent: Option<String>,

Use triple slash doc comment, so that IDE is able to display the explanation of these fields.

---

src/meta/app/src/principal/user_token.rs line 23 at r4 (raw file):

)]
pub enum TokenType {
    // refresh token, with a longer TTL, is only used for auth when get new refresh token and session token.

Is this explanation correct?

Suggestion:

    /// A token that grants the privilege to a user for refreshing `Session` and `Refresh` token,
    /// with a longer TTL than `Session` token so that when `Session` token expires,
    /// user is still able to obtain a refreshed token with this one.

---

src/query/service/src/servers/http/v1/session/token_manager.rs line 48 at r4 (raw file):

/// by adding SESSION_TOKEN_VALIDITY_IN_SECS, avoid touch refresh token for each request.
/// note the ttl is not accurate, the TTL is in fact longer (by 0 to 1 hour) then expected.
const REFRESH_TOKEN_TTL: u64 = REFRESH_TOKEN_VALIDITY_IN_SECS + SESSION_TOKEN_VALIDITY_IN_SECS;

Always use std::time::Duration instead if possible.

Code quote:

/// target TTL
pub const REFRESH_TOKEN_VALIDITY_IN_SECS: u64 = 3600 * 4; // 4 hours
pub const SESSION_TOKEN_VALIDITY_IN_SECS: u64 = 3600; // 1 hour

/// to cove network latency, retry and time skew
const TOKEN_TTL_DELAY: u64 = 300;
/// in case of client retry, shorted the ttl instead of drop at once
/// only required for refresh token.
const TOKEN_DROP_DELAY: u64 = 90;

/// by adding SESSION_TOKEN_VALIDITY_IN_SECS, avoid touch refresh token for each request.
/// note the ttl is not accurate, the TTL is in fact longer (by 0 to 1 hour) then expected.
const REFRESH_TOKEN_TTL: u64 = REFRESH_TOKEN_VALIDITY_IN_SECS + SESSION_TOKEN_VALIDITY_IN_SECS;

---

src/query/service/src/servers/http/v1/session/token_manager.rs line 91 at r4 (raw file):

        // those infos are set when the request is authed
        let tenant = session.get_current_tenant();
        let tenant_name = tenant.tenant.to_string();

Use method tenant_name() to get the string name, the internal structure may be refactored in future.

Suggestion:

        let tenant_name = tenant.tenant_name().to_string();

---

src/query/service/src/servers/http/v1/session/token_manager.rs line 107 at r4 (raw file):

        let meta_store = UserApiProvider::instance();

        let now = unix_ts();

Make unix_ts() return a Duration, convert to integer only when Duration can not be used.

---

src/query/users/src/user_token.rs line 52 at r4 (raw file):

        token_api_provider.drop_token(token_hash).await
    }
}

These methods are not very necessary. They can be replaced with indirect calls such as: UserApiProvider::token_api().upsert_token().

Code quote:

impl UserApiProvider {
    #[async_backtrace::framed]
    pub async fn upsert_token(
        &self,
        tenant: &Tenant,
        token_hash: &str,
        token_info: QueryTokenInfo,
        ttl_in_secs: u64,
        update_only: bool,
    ) -> Result<bool> {
        let token_api_provider = self.token_api(tenant);
        token_api_provider
            .upsert_token(token_hash, token_info, ttl_in_secs, update_only)
            .await
    }

    #[async_backtrace::framed]
    pub async fn get_token(
        &self,
        tenant: &Tenant,
        token_hash: &str,
    ) -> Result<Option<QueryTokenInfo>> {
        let token_api_provider = self.token_api(tenant);
        token_api_provider.get_token(token_hash).await
    }

    #[async_backtrace::framed]
    pub async fn drop_token(&self, tenant: &Tenant, token_hash: &str) -> Result<()> {
        let token_api_provider = self.token_api(tenant);
        token_api_provider.drop_token(token_hash).await
    }
}

---

src/query/service/src/servers/http/v1/session/login_handler.rs line 39 at r4 (raw file):

Previously, youngsofun (Yang Xiufeng) wrote…

version may be still helpful when error?

This struct can be replaced with Result<OKResponse, QueryError>, where OKResponse includes version, session_id etc. Since QueryError seems like a for human error the version can be embedded in QueryError.mesage or QueryError.detail as string.

[youngsofun]

Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @drmingdrmer, @flaneur2020, @hantmac, and @sundy-li)

---

src/query/service/src/servers/http/v1/session/token_manager.rs line 48 at r4 (raw file):

Previously, drmingdrmer (张炎泼) wrote…

Always use std::time::Duration instead if possible.

Done.

---

src/query/service/src/servers/http/v1/session/token_manager.rs line 107 at r4 (raw file):

Previously, drmingdrmer (张炎泼) wrote…

Make unix_ts() return a Duration, convert to integer only when Duration can not be used.

working

---

src/query/users/src/user_token.rs line 52 at r4 (raw file):

Previously, drmingdrmer (张炎泼) wrote…

These methods are not very necessary. They can be replaced with indirect calls such as: UserApiProvider::token_api().upsert_token().

working

---

src/query/management/src/token/token_mgr.rs line 64 at r1 (raw file):

Previously, drmingdrmer (张炎泼) wrote…

is_update here is not very clear about its purpose:
replace_existing would be better reflecting its behavior.

Avoid using plain number duration. Use Duration type for ttl.

renamed to update_only

[drmingdrmer]

Reviewed 6 of 7 files at r5, 3 of 3 files at r6, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @b41sh, @flaneur2020, @hantmac, @sundy-li, and @youngsofun)

---

src/query/service/src/servers/http/v1/session/renew_handler.rs line 33 at r4 (raw file):

Previously, youngsofun (Yang Xiufeng) wrote…

done

Why not just use Result? 🤔

---

src/query/service/src/servers/http/v1/session/login_handler.rs line 39 at r4 (raw file):

Previously, youngsofun (Yang Xiufeng) wrote…

done

Why not make LoginResponse a Result? It will be able to reuse all Result's methods.

[youngsofun]

@drmingdrmer

not need methods of Result，it will only be used to dump json
but need serde（untaged）

[drmingdrmer]

Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @b41sh, @flaneur2020, @hantmac, @sundy-li, and @youngsofun)

[drmingdrmer]

Reviewed 1 of 1 files at r8, all commit messages.
Reviewable status: all files reviewed, 15 unresolved discussions (waiting on @b41sh, @flaneur2020, @sundy-li, and @youngsofun)

[drmingdrmer]

Reviewed 5 of 5 files at r9, all commit messages.
Reviewable status: all files reviewed, 15 unresolved discussions (waiting on @b41sh, @flaneur2020, @sundy-li, and @youngsofun)

[drmingdrmer]

Reviewed 5 of 5 files at r10, all commit messages.
Reviewable status: all files reviewed, 15 unresolved discussions (waiting on @b41sh, @flaneur2020, @sundy-li, and @youngsofun)

[drmingdrmer]

Reviewed 1 of 1 files at r11, 7 of 7 files at r12, all commit messages.
Reviewable status: all files reviewed, 15 unresolved discussions (waiting on @b41sh, @flaneur2020, @sundy-li, and @youngsofun)

[youngsofun]

@flaneur2020 @BohuTANG what about merge this first, so we can begin the develop of clients, we still have chance to modify details before all is ready