Skip to content

Commit 5adcd27

Browse files
kevintang2022kevintang2022
authored
Presto add sqlText to accessControlContext (#26054)
## Description <!---Describe your changes in detail--> Currently, sqlText is not accessible to AccessControlContext. This change allow the sqlText to be accessible from the system access control APIs called within AccessControlUtils.java Differential Revision: D82132353 ## Motivation and Context <!---Why is this change required? What problem does it solve?--> <!---If it fixes an open issue, please link to the issue here.--> sqlText is currently inaccessible from within system access control APIs, such as selectAuthorizedIdentity and checkCanSetUser ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> - Add sqlText to SessionContext implementations - Add sqlText to AccessControlContext, which is gated behind an opt-in config feature ## Test Plan <!---Please fill in how you tested your change--> AccessControlUtils is able to access sqlText in the tests. ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [x] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == General Changes * Add sqlText to SessionContext to be used by system access control APIs ```
1 parent bc7ac8f commit 5adcd27

File tree

10 files changed

+105
-12
lines changed

10 files changed

+105
-12
lines changed

‎presto-main-base/src/main/java/com/facebook/presto/security/AccessControlUtils.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ public static void checkPermissions(AccessControl accessControl, SecurityConfig
4949
sessionContext.getRuntimeStats(),
5050
Optional.empty(),
5151
Optional.ofNullable(sessionContext.getCatalog()),
52-
Optional.ofNullable(sessionContext.getSchema())),
52+
Optional.ofNullable(sessionContext.getSchema()),
53+
getSqlText(sessionContext, securityConfig)),
5354
identity.getPrincipal(),
5455
identity.getUser());
5556
}
@@ -77,11 +78,20 @@ public static Optional<AuthorizedIdentity> getAuthorizedIdentity(AccessControl a
7778
sessionContext.getRuntimeStats(),
7879
Optional.empty(),
7980
Optional.ofNullable(sessionContext.getCatalog()),
80-
Optional.ofNullable(sessionContext.getSchema())),
81+
Optional.ofNullable(sessionContext.getSchema()),
82+
getSqlText(sessionContext, securityConfig)),
8183
identity.getUser(),
8284
sessionContext.getCertificates());
8385
return Optional.of(authorizedIdentity);
8486
}
8587
return Optional.empty();
8688
}
89+
90+
private static Optional<String> getSqlText(SessionContext sessionContext, SecurityConfig securityConfig)
91+
{
92+
if (securityConfig.isEnableSqlQueryTextContextField()) {
93+
return Optional.of(sessionContext.getSqlText());
94+
}
95+
return Optional.empty();
96+
}
8797
}

‎presto-main-base/src/main/java/com/facebook/presto/server/SessionContext.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ default List<X509Certificate> getCertificates()
5050
@Nullable
5151
String getSchema();
5252

53+
String getSqlText();
54+
5355
@Nullable
5456
String getSource();
5557

‎presto-main-base/src/main/java/com/facebook/presto/server/security/SecurityConfig.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ public class SecurityConfig
3333
private List<AuthenticationType> authenticationTypes = ImmutableList.of();
3434
private boolean allowForwardedHttps;
3535
private boolean authorizedIdentitySelectionEnabled;
36+
private boolean enableSqlQueryTextContextField;
3637

3738
public enum AuthenticationType
3839
{
@@ -97,4 +98,17 @@ public boolean isAuthorizedIdentitySelectionEnabled()
9798
{
9899
return authorizedIdentitySelectionEnabled;
99100
}
101+
102+
@Config("permissions.enable-sql-query-text-context-field")
103+
@ConfigDescription("Allow sql query text to be stored inside access control context")
104+
public SecurityConfig setEnableSqlQueryTextContextField(boolean enableSqlQueryTextContextField)
105+
{
106+
this.enableSqlQueryTextContextField = enableSqlQueryTextContextField;
107+
return this;
108+
}
109+
110+
public boolean isEnableSqlQueryTextContextField()
111+
{
112+
return enableSqlQueryTextContextField;
113+
}
100114
}

‎presto-main-base/src/test/java/com/facebook/presto/server/security/TestSecurityConfig.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ public void testDefaults()
3131
ConfigAssertions.assertRecordedDefaults(ConfigAssertions.recordDefaults(SecurityConfig.class)
3232
.setAuthenticationTypes("")
3333
.setAllowForwardedHttps(false)
34-
.setAuthorizedIdentitySelectionEnabled(false));
34+
.setAuthorizedIdentitySelectionEnabled(false)
35+
.setEnableSqlQueryTextContextField(false));
3536
}
3637

3738
@Test
@@ -41,12 +42,14 @@ public void testExplicitPropertyMappings()
4142
.put("http-server.authentication.type", "KERBEROS,PASSWORD")
4243
.put("http-server.authentication.allow-forwarded-https", "true")
4344
.put("permissions.authorized-identity-selection-enabled", "true")
45+
.put("permissions.enable-sql-query-text-context-field", "true")
4446
.build();
4547

4648
SecurityConfig expected = new SecurityConfig()
4749
.setAuthenticationTypes(ImmutableList.of(KERBEROS, PASSWORD))
4850
.setAllowForwardedHttps(true)
49-
.setAuthorizedIdentitySelectionEnabled(true);
51+
.setAuthorizedIdentitySelectionEnabled(true)
52+
.setEnableSqlQueryTextContextField(true);
5053

5154
ConfigAssertions.assertFullMapping(properties, expected);
5255
}

‎presto-main/src/main/java/com/facebook/presto/server/HttpRequestSessionContext.java‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@ public final class HttpRequestSessionContext
9999
private final String catalog;
100100
private final String schema;
101101

102+
private final String sqlText;
103+
102104
private final Identity identity;
103105
private final Optional<AuthorizedIdentity> authorizedIdentity;
104106
private final List<X509Certificate> certificates;
@@ -128,7 +130,7 @@ public final class HttpRequestSessionContext
128130

129131
public HttpRequestSessionContext(HttpServletRequest servletRequest, SqlParserOptions sqlParserOptions)
130132
{
131-
this(servletRequest, sqlParserOptions, NoopTracerProvider.NOOP_TRACER_PROVIDER, Optional.empty());
133+
this(servletRequest, sqlParserOptions, NoopTracerProvider.NOOP_TRACER_PROVIDER, Optional.empty(), "");
132134
}
133135

134136
/**
@@ -138,13 +140,16 @@ public HttpRequestSessionContext(HttpServletRequest servletRequest, SqlParserOpt
138140
* @param sessionPropertyManager is used to provide with some default session values. In some scenarios we need
139141
* those default values even before session for a query is created. This is how we can get it at this
140142
* session context creation stage.
143+
* @param sqlText query string
141144
* @throws WebApplicationException
142145
*/
143-
public HttpRequestSessionContext(HttpServletRequest servletRequest, SqlParserOptions sqlParserOptions, TracerProvider tracerProvider, Optional<SessionPropertyManager> sessionPropertyManager)
146+
public HttpRequestSessionContext(HttpServletRequest servletRequest, SqlParserOptions sqlParserOptions, TracerProvider tracerProvider, Optional<SessionPropertyManager> sessionPropertyManager, String sqlText)
144147
throws WebApplicationException
145148
{
146149
catalog = trimEmptyToNull(servletRequest.getHeader(PRESTO_CATALOG));
147150
schema = trimEmptyToNull(servletRequest.getHeader(PRESTO_SCHEMA));
151+
this.sqlText = requireNonNull(sqlText, "sqlText is null");
152+
148153
assertRequest((catalog != null) || (schema == null), "Schema is set but catalog is not");
149154

150155
String user = trimEmptyToNull(servletRequest.getHeader(PRESTO_USER));
@@ -431,6 +436,12 @@ public String getSchema()
431436
return schema;
432437
}
433438

439+
@Override
440+
public String getSqlText()
441+
{
442+
return sqlText;
443+
}
444+
434445
@Override
435446
public String getSource()
436447
{

‎presto-main/src/main/java/com/facebook/presto/server/protocol/QueuedStatementResource.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,8 @@ public Response postStatement(
251251
servletRequest,
252252
sqlParserOptions,
253253
tracerProviderManager.getTracerProvider(),
254-
Optional.of(sessionPropertyManager));
254+
Optional.of(sessionPropertyManager),
255+
statement);
255256
QueryId newQueryId = dispatchManager.createQueryId();
256257
Query query = new Query(
257258
statement,
@@ -324,7 +325,8 @@ public Response putStatement(
324325
servletRequest,
325326
sqlParserOptions,
326327
tracerProviderManager.getTracerProvider(),
327-
Optional.of(sessionPropertyManager));
328+
Optional.of(sessionPropertyManager),
329+
statement);
328330
Query attemptedQuery = new Query(statement, sessionContext, dispatchManager, executingQueryResponseProvider, 0, queryId, slug);
329331
Query query = queries.computeIfAbsent(queryId, unused -> attemptedQuery);
330332

‎presto-spark-base/src/main/java/com/facebook/presto/spark/PrestoSparkQueryExecutionFactory.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -609,7 +609,8 @@ public IPrestoSparkQueryExecution create(
609609
SessionContext sessionContext = PrestoSparkSessionContext.createFromSessionInfo(
610610
prestoSparkSession,
611611
credentialsProviders,
612-
authenticatorProviders);
612+
authenticatorProviders,
613+
sql);
613614

614615
SessionBuilder sessionBuilder = sessionSupplier.createSessionBuilder(queryId, sessionContext, warningCollectorFactory);
615616
sessionPropertyDefaults.applyDefaultProperties(sessionBuilder, Optional.empty(), Optional.empty());

‎presto-spark-base/src/main/java/com/facebook/presto/spark/PrestoSparkSessionContext.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ public class PrestoSparkSessionContext
4343
private final String schema;
4444
private final String source;
4545

46+
private final String sqlText;
47+
4648
private final String userAgent;
4749
private final String clientInfo;
4850
private final Set<String> clientTags;
@@ -57,7 +59,8 @@ public class PrestoSparkSessionContext
5759
public static PrestoSparkSessionContext createFromSessionInfo(
5860
PrestoSparkSession prestoSparkSession,
5961
Set<PrestoSparkCredentialsProvider> credentialsProviders,
60-
Set<PrestoSparkAuthenticatorProvider> authenticatorProviders)
62+
Set<PrestoSparkAuthenticatorProvider> authenticatorProviders,
63+
String sqlQueryText)
6164
{
6265
ImmutableMap.Builder<String, String> extraCredentials = ImmutableMap.builder();
6366
extraCredentials.putAll(prestoSparkSession.getExtraCredentials());
@@ -78,6 +81,7 @@ public static PrestoSparkSessionContext createFromSessionInfo(
7881
prestoSparkSession.getCatalog().orElse(null),
7982
prestoSparkSession.getSchema().orElse(null),
8083
prestoSparkSession.getSource().orElse(null),
84+
sqlQueryText,
8185
prestoSparkSession.getUserAgent().orElse(null),
8286
prestoSparkSession.getClientInfo().orElse(null),
8387
prestoSparkSession.getClientTags(),
@@ -93,6 +97,7 @@ public PrestoSparkSessionContext(
9397
String catalog,
9498
String schema,
9599
String source,
100+
String sqlText,
96101
String userAgent,
97102
String clientInfo,
98103
Set<String> clientTags,
@@ -106,6 +111,7 @@ public PrestoSparkSessionContext(
106111
this.catalog = catalog;
107112
this.schema = schema;
108113
this.source = source;
114+
this.sqlText = requireNonNull(sqlText, "sqlText is null");
109115
this.userAgent = userAgent;
110116
this.clientInfo = clientInfo;
111117
this.clientTags = ImmutableSet.copyOf(requireNonNull(clientTags, "clientTags is null"));
@@ -136,6 +142,12 @@ public String getSchema()
136142
return schema;
137143
}
138144

145+
@Override
146+
public String getSqlText()
147+
{
148+
return sqlText;
149+
}
150+
139151
@Nullable
140152
@Override
141153
public String getSource()

‎presto-spi/src/main/java/com/facebook/presto/spi/security/AccessControlContext.java‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ public class AccessControlContext
3535
private final Optional<QueryType> queryType;
3636
private final Optional<String> catalog;
3737
private final Optional<String> schema;
38+
private final Optional<String> sqlText;
3839

3940
public AccessControlContext(
4041
QueryId queryId,
@@ -46,6 +47,30 @@ public AccessControlContext(
4647
Optional<QueryType> queryType,
4748
Optional<String> catalog,
4849
Optional<String> schema)
50+
{
51+
this(queryId,
52+
clientInfo,
53+
clientTags,
54+
source,
55+
warningCollector,
56+
runtimeStats,
57+
queryType,
58+
catalog,
59+
schema,
60+
Optional.empty());
61+
}
62+
63+
public AccessControlContext(
64+
QueryId queryId,
65+
Optional<String> clientInfo,
66+
Set<String> clientTags,
67+
Optional<String> source,
68+
WarningCollector warningCollector,
69+
RuntimeStats runtimeStats,
70+
Optional<QueryType> queryType,
71+
Optional<String> catalog,
72+
Optional<String> schema,
73+
Optional<String> sqlText)
4974
{
5075
this.queryId = requireNonNull(queryId, "queryId is null");
5176
this.clientInfo = requireNonNull(clientInfo, "clientInfo is null");
@@ -56,6 +81,7 @@ public AccessControlContext(
5681
this.queryType = requireNonNull(queryType, "queryType is null");
5782
this.catalog = requireNonNull(catalog, "catalog is null");
5883
this.schema = requireNonNull(schema, "schema is null");
84+
this.sqlText = requireNonNull(sqlText, "sqlText is null");
5985
}
6086

6187
public QueryId getQueryId()
@@ -103,10 +129,15 @@ public Optional<String> getSchema()
103129
return schema;
104130
}
105131

132+
public Optional<String> getSqlText()
133+
{
134+
return sqlText;
135+
}
136+
106137
@Override
107138
public int hashCode()
108139
{
109-
return Objects.hash(queryId, clientInfo, clientTags, source, queryType, catalog, schema);
140+
return Objects.hash(queryId, clientInfo, clientTags, source, queryType, catalog, schema, sqlText);
110141
}
111142

112143
@Override
@@ -125,6 +156,7 @@ public boolean equals(Object obj)
125156
Objects.equals(this.source, other.source) &&
126157
Objects.equals(this.queryType, other.queryType) &&
127158
Objects.equals(this.catalog, other.catalog) &&
128-
Objects.equals(this.schema, other.schema);
159+
Objects.equals(this.schema, other.schema) &&
160+
Objects.equals(this.sqlText, other.sqlText);
129161
}
130162
}

‎presto-tests/src/test/java/com/facebook/presto/execution/TestingSessionContext.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,12 @@ public String getSchema()
6060
return session.getSchema().orElse(null);
6161
}
6262

63+
@Override
64+
public String getSqlText()
65+
{
66+
return session.getAccessControlContext().getSqlText().orElse("");
67+
}
68+
6369
@Override
6470
public String getSource()
6571
{

0 commit comments

Comments
 (0)