feat(cubesql): Provide password supplied by Postgres connection as a 3rd argument of check_sql_auth() (#7471)

* feat(cubesql): Provide password supplied by Postgres connection as a 3rd argument of `check_sql_auth()`

Fixes #5430

* Add test for password argument

* Add authentication expiration

* Add docs

* Add explicit skip_password_check flag for dev mode, close connection on re-authentication failure, strip CubeError onion of Internal errors

* Fix python test

* Remove another `Internal:` from snapshot

* Remove another `Internal:` from test

* Fix docs

Author: paveltiunov

docs/pages/reference/configuration/config.mdx

@@ -1085,20 +1085,27 @@ implementation verifies user name and password from environment variables:
 `CUBEJS_SQL_USER`, `CUBEJS_SQL_PASSWORD`, but in [development
 mode][ref-development-mode] it ignores validation.
 
-Called on each request from Cube SQL API.
+Called on each new connection to Cube SQL API, on change user by `SET USER` or `__user` field, every `CUBESQL_AUTH_EXPIRE_SECS`.
 
 For example, you can use `check_sql_auth` to validate username and password.
+`password` argument is provided only when new connections are established.
+`check_sql_auth` implementation should gracefully handle missing `password` field to handle change user and re-authentication flows.
+`check_sql_auth` should always return `password` as it used for validation of password provided by user.
+If clear text password can't be obtained, best practice is to return `password` provided as an argument after password validation.
+Only security context is used for change user and re-authentication flows so returned `password` isn't checked in this case.
 
 <CodeTabs>
 
 ```python
 from cube import config
 
 @config('check_sql_auth')
-def check_sql_auth(req: dict, user_name: str) -> dict:
+def check_sql_auth(req: dict, user_name: str, password: str) -> dict:
   if user_name == 'my_user':
+    if password and password != 'my_password':
+      raise Exception('Access denied')
     return {
-      'password': 'my_password',
+      'password': password,
       'securityContext': {
         'some': 'data'
       }
@@ -1110,10 +1117,13 @@ def check_sql_auth(req: dict, user_name: str) -> dict:
 
 ```javascript
 module.exports = {
-  checkSqlAuth: (req, user_name) => {
+  checkSqlAuth: (req, user_name, password) => {
     if (user_name === 'my_user') {
+      if (password && password !== 'my_password') {
+        throw new Error('Access denied');
+      }
       return {
-        password: 'my_password',
+        password,
         securityContext: {
           some: 'data'
         },

docs/pages/reference/configuration/environment-variables.mdx

@@ -975,6 +975,15 @@ If `true`, then send telemetry to Cube.
 | --------------- | ---------------------- | --------------------- |
 | `true`, `false` | `true`                 | `true`                |
 
+
+## `CUBESQL_AUTH_EXPIRE_SECS`
+
+Number of seconds before session's SQL API security context will be invalidated.
+
+| Possible Values | Default in Development | Default in Production |
+| --------------- | ---------------------- | --------------------- |
+| A valid integer number | `300`                 | `300`                |
+
 ## `CUBESTORE_METRICS_FORMAT`
 
 Define which metrics collector format.

packages/cubejs-api-gateway/src/sql-server.ts

@@ -47,20 +47,21 @@ export class SQLServer {
 
     const contextByRequest = async (request, session) => {
       let userForContext = session.user;
+      let { securityContext } = session;
 
       if (request.meta.changeUser && request.meta.changeUser !== session.user) {
         const canSwitch = session.superuser || await canSwitchSqlUser(session.user, request.meta.changeUser);
         if (canSwitch) {
           userForContext = request.meta.changeUser;
+          const current = await checkSqlAuth(request, userForContext, null);
+          securityContext = current.securityContext;
         } else {
           throw new Error(
             `You cannot change security context via __user from ${session.user} to ${request.meta.changeUser}, because it's not allowed.`
           );
         }
       }
-      // @todo Store security context in native for session's user, but not for switching
-      const current = await checkSqlAuth(request, userForContext);
-      return this.contextByNativeReq(request, current.securityContext, request.id);
+      return this.contextByNativeReq(request, securityContext, request.id);
     };
 
     const canSwitchUserForSession = async (session, user) => session.superuser || canSwitchSqlUser(session.user, user);
@@ -69,19 +70,19 @@ export class SQLServer {
       port: options.sqlPort,
       pgPort: options.pgSqlPort,
       nonce: options.sqlNonce,
-      checkAuth: async ({ request, user }) => {
-        const { password, superuser } = await checkSqlAuth(request, user);
+      checkAuth: async ({ request, user, password }) => {
+        const { password: returnedPassword, superuser, securityContext, skipPasswordCheck } = await checkSqlAuth(request, user, password);
 
         // Strip securityContext to improve speed deserialization
         return {
-          password,
+          password: returnedPassword,
           superuser: superuser || false,
+          securityContext,
+          skipPasswordCheck,
         };
       },
       meta: async ({ request, session }) => {
-        // @todo Store security context in native
-        const { securityContext } = await checkSqlAuth(request, session.user);
-        const context = await this.apiGateway.contextByReq(<any> request, securityContext, request.id);
+        const context = await this.apiGateway.contextByReq(<any> request, session.securityContext, request.id);
 
         // eslint-disable-next-line no-async-promise-executor
         return new Promise(async (resolve, reject) => {
@@ -177,9 +178,7 @@ export class SQLServer {
       sqlGenerators: async (paramsJson: string) => {
         // TODO get rid of it
         const { request, session } = JSON.parse(paramsJson);
-        // @todo Store security context in native
-        const { securityContext } = await checkSqlAuth(request, session.user);
-        const context = await this.apiGateway.contextByReq(<any> request, securityContext, request.id);
+        const context = await this.apiGateway.contextByReq(<any> request, session.securityContext, request.id);
 
         // eslint-disable-next-line no-async-promise-executor
         return new Promise(async (resolve, reject) => {
@@ -200,16 +199,12 @@ export class SQLServer {
   }
 
   protected wrapCheckSqlAuthFn(checkSqlAuth: CheckSQLAuthFn): CheckSQLAuthFn {
-    return async (req, user) => {
-      const response = await checkSqlAuth(req, user);
-      if (typeof response !== 'object' || response.password === null) {
+    return async (req, user, password) => {
+      const response = await checkSqlAuth(req, user, password);
+      if (typeof response !== 'object') {
         throw new Error('checkSqlAuth must return an object');
       }
 
-      if (!response.password) {
-        throw new Error('checkSqlAuth must return an object with password field');
-      }
-
       return response;
     };
   }
@@ -255,7 +250,8 @@ export class SQLServer {
 
       return {
         password: allowedPassword,
-        securityContext: {}
+        securityContext: {},
+        skipPasswordCheck: getEnv('devMode') && !allowedPassword
       };
     };
   }

packages/cubejs-api-gateway/src/types/auth.ts

@@ -55,7 +55,8 @@ type CheckAuthFn =
 type CheckSQLAuthSuccessResponse = {
   password: string | null,
   superuser?: boolean,
-  securityContext?: any
+  securityContext?: any,
+  skipPasswordCheck?: boolean,
 };
 
 /**
@@ -64,7 +65,7 @@ type CheckSQLAuthSuccessResponse = {
  * auth logic.
  */
 type CheckSQLAuthFn =
-  (ctx: any, user: string | null) =>
+  (ctx: any, user: string | null, password: string | null) =>
     Promise<CheckSQLAuthSuccessResponse> |
     CheckSQLAuthSuccessResponse;
 

packages/cubejs-backend-native/js/index.ts

@@ -24,17 +24,21 @@ export interface Request<Meta> {
 
 export interface CheckAuthResponse {
     password: string | null,
-    superuser: boolean
+    superuser: boolean,
+    securityContext: any,
+    skipPasswordCheck?: boolean,
 }
 
 export interface CheckAuthPayload {
     request: Request<undefined>,
-    user: string | null
+    user: string | null,
+    password: string | null,
 }
 
 export interface SessionContext {
     user: string | null,
     superuser: boolean,
+    securityContext: any,
 }
 
 export interface LoadPayload {

packages/cubejs-backend-native/src/auth.rs

@@ -40,18 +40,24 @@ pub struct TransportRequest {
 struct CheckAuthRequest {
     request: TransportRequest,
     user: Option<String>,
+    password: Option<String>,
 }
 
 #[derive(Debug, Deserialize)]
 struct CheckAuthResponse {
     password: Option<String>,
     superuser: bool,
+    #[serde(rename = "securityContext", skip_serializing_if = "Option::is_none")]
+    security_context: Option<serde_json::Value>,
+    #[serde(rename = "skipPasswordCheck", skip_serializing_if = "Option::is_none")]
+    skip_password_check: Option<bool>,
 }
 
 #[derive(Debug)]
 pub struct NativeAuthContext {
     pub user: Option<String>,
     pub superuser: bool,
+    pub security_context: Option<serde_json::Value>,
 }
 
 impl AuthContext for NativeAuthContext {
@@ -62,7 +68,11 @@ impl AuthContext for NativeAuthContext {
 
 #[async_trait]
 impl SqlAuthService for NodeBridgeAuthService {
-    async fn authenticate(&self, user: Option<String>) -> Result<AuthenticateResponse, CubeError> {
+    async fn authenticate(
+        &self,
+        user: Option<String>,
+        password: Option<String>,
+    ) -> Result<AuthenticateResponse, CubeError> {
         trace!("[auth] Request ->");
 
         let request_id = Uuid::new_v4().to_string();
@@ -73,6 +83,7 @@ impl SqlAuthService for NodeBridgeAuthService {
                 meta: None,
             },
             user: user.clone(),
+            password: password.clone(),
         })?;
         let response: CheckAuthResponse = call_js_with_channel_as_callback(
             self.channel.clone(),
@@ -86,8 +97,10 @@ impl SqlAuthService for NodeBridgeAuthService {
             context: Arc::new(NativeAuthContext {
                 user,
                 superuser: response.superuser,
+                security_context: response.security_context,
             }),
             password: response.password,
+            skip_password_check: response.skip_password_check.unwrap_or(false),
         })
     }
 }

packages/cubejs-backend-native/src/transport.rs

@@ -61,6 +61,8 @@ impl NodeBridgeTransport {
 struct SessionContext {
     user: Option<String>,
     superuser: bool,
+    #[serde(rename = "securityContext", skip_serializing_if = "Option::is_none")]
+    security_context: Option<serde_json::Value>,
 }
 
 #[derive(Debug, Serialize)]
@@ -113,6 +115,7 @@ impl TransportService for NodeBridgeTransport {
             session: SessionContext {
                 user: native_auth.user.clone(),
                 superuser: native_auth.superuser,
+                security_context: native_auth.security_context.clone(),
             },
         })?;
         let response = call_js_with_channel_as_callback::<V1MetaResponse>(
@@ -206,6 +209,7 @@ impl TransportService for NodeBridgeTransport {
             session: SessionContext {
                 user: native_auth.user.clone(),
                 superuser: native_auth.superuser,
+                security_context: native_auth.security_context.clone(),
             },
             sql_query: None,
             member_to_alias,
@@ -280,6 +284,7 @@ impl TransportService for NodeBridgeTransport {
                 session: SessionContext {
                     user: native_auth.user.clone(),
                     superuser: native_auth.superuser,
+                    security_context: native_auth.security_context.clone(),
                 },
                 sql_query: sql_query.clone().map(|q| (q.sql, q.values)),
                 member_to_alias: None,
@@ -358,6 +363,7 @@ impl TransportService for NodeBridgeTransport {
                 session: SessionContext {
                     user: native_auth.user.clone(),
                     superuser: native_auth.superuser,
+                    security_context: native_auth.security_context.clone(),
                 },
                 member_to_alias: None,
                 expression_params: None,
@@ -402,6 +408,7 @@ impl TransportService for NodeBridgeTransport {
                 session: SessionContext {
                     user: native_auth.user.clone(),
                     superuser: native_auth.superuser,
+                    security_context: native_auth.security_context.clone(),
                 },
             },
             Box::new(|cx, v| match NodeObjSerializer::serialize(&v, cx) {

packages/cubejs-backend-native/test/__snapshots__/jinja.test.ts.snap

@@ -247,7 +247,7 @@ dump:
 `;
 
 exports[`Jinja (new api) render template_error_python.jinja: template_error_python.jinja 1`] = `
-[Error: could not render block: Call error: Internal: Python error: Exception: Random Exception
+[Error: could not render block: Call error: Python error: Exception: Random Exception
 Traceback (most recent call last):
   File "jinja-instance.py", line 110, in throw_exception
 

packages/cubejs-backend-native/test/sql.test.ts

@@ -31,6 +31,7 @@ describe('SQLInterface', () => {
       expect(session).toEqual({
         user: expect.toBeTypeOrNull(String),
         superuser: expect.any(Boolean),
+        securityContext: { foo: 'bar' }
       });
 
       // It's just an emulation that ApiGateway returns error
@@ -56,6 +57,7 @@ describe('SQLInterface', () => {
       expect(session).toEqual({
         user: expect.toBeTypeOrNull(String),
         superuser: expect.any(Boolean),
+        securityContext: { foo: 'bar' }
       });
 
       // It's just an emulation that ApiGateway returns error
@@ -98,6 +100,7 @@ describe('SQLInterface', () => {
       expect(session).toEqual({
         user: expect.toBeTypeOrNull(String),
         superuser: expect.any(Boolean),
+        securityContext: { foo: 'bar' },
       });
 
       return metaFixture;
@@ -125,13 +128,15 @@ describe('SQLInterface', () => {
         return {
           password: 'password_for_allowed_user',
           superuser: false,
+          securityContext: { foo: 'bar' },
         };
       }
 
       if (user === 'admin') {
         return {
           password: 'password_for_admin',
           superuser: true,
+          securityContext: { foo: 'admin' },
         };
       }
 
@@ -175,6 +180,7 @@ describe('SQLInterface', () => {
             meta: null,
           },
           user: user || null,
+          password: null,
         });
       };
 
@@ -226,6 +232,7 @@ describe('SQLInterface', () => {
           meta: null,
         },
         user: 'allowed_user',
+        password: null,
       });
 
       expect(meta.mock.calls.length).toEqual(1);
@@ -237,6 +244,7 @@ describe('SQLInterface', () => {
         session: {
           user: 'allowed_user',
           superuser: false,
+          securityContext: { foo: 'bar' },
         }
       });
 

packages/cubejs-testing/birdbox-fixtures/postgresql/single/sqlapi.js

@@ -12,10 +12,13 @@ module.exports = {
 
     return query;
   },
-  checkSqlAuth: async (req, user) => {
+  checkSqlAuth: async (req, user, password) => {
     if (user === 'admin') {
+      if (password && password !== 'admin_password') {
+        throw new Error(`Password doesn't match for ${user}`);
+      }
       return {
-        password: 'admin_password',
+        password,
         superuser: true,
         securityContext: {
           user: 'admin'