From bcba9d3415c8657c8b3e11560d3bef762a5b7928 Mon Sep 17 00:00:00 2001 From: Arpit Mohan Date: Tue, 17 Dec 2019 09:28:59 +0530 Subject: [PATCH] Creating a list of public endpoints that anonymous users can access. OPA controls access to all endpoints and the list of authenticated resources and public URLs is defined in a single place in that file. The url_allow function in acl.rego is an overloaded function that replicates the OR condition in Rego. Either the user is authenticated and has permissions to access those resources, or the URL is public and accessible by any user. --- .../com/appsmith/server/acl/AclService.java | 11 +++++++--- .../server/configurations/SecurityConfig.java | 2 ++ .../appsmith/server/filters/AclFilter.java | 5 +++-- .../resources/public/appsmith/authz/acl.rego | 20 ++++++++++++++++-- .../src/main/resources/public/bundle.tar.gz | Bin 1183 -> 1261 bytes 5 files changed, 31 insertions(+), 7 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/acl/AclService.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/acl/AclService.java index b507a4b8f3..3b717f1c0c 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/acl/AclService.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/acl/AclService.java @@ -39,20 +39,26 @@ public class AclService { this.groupService = groupService; } - public Mono evaluateAcl(HttpMethod httpMethod, String resource) { + public Mono evaluateAcl(HttpMethod httpMethod, String resource, String requestUrl) { JSONObject requestBody = new JSONObject(); JSONObject input = new JSONObject(); JSONObject jsonUser = new JSONObject(); input.put("user", jsonUser); input.put("method", httpMethod.name()); input.put("resource", resource); + input.put("url", requestUrl); // The url is required for OPA to gate keep only public URLs for anonymous users requestBody.put("input", input); Mono user = sessionUserService.getCurrentUser(); return user - .map(u -> { + // This is when the user doesn't have an existing session. The user is anonymous + .switchIfEmpty(Mono.defer(() -> { + User anonymous = new User(); + return Mono.just(anonymous); + })) + .flatMap(u -> { Set globalPermissions = new HashSet<>(); Set groupSet = u.getGroupIds(); globalPermissions.addAll(u.getPermissions()); @@ -62,7 +68,6 @@ public class AclService { .collectList() .thenReturn(globalPermissions); }) - .flatMap(obj -> obj) .flatMap(permissions -> { jsonUser.put("permissions", permissions); String finalUrl = url + pkgName; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java index 5e4dda8350..e11df39307 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java @@ -82,6 +82,8 @@ public class SecurityConfig { .cors().and() .csrf().disable() .authorizeExchange() + // All public URLs that should be served to anonymous users should be defined in acl.rego file + // This list of matchers is only the list of URLs that shouldn't return 401 unauthorized .matchers(ServerWebExchangeMatchers.pathMatchers(HttpMethod.GET, "/login")) .permitAll() .pathMatchers("/public/**").permitAll() diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/AclFilter.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/AclFilter.java index d79dcb11a7..6338358023 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/AclFilter.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/AclFilter.java @@ -44,7 +44,8 @@ public class AclFilter implements WebFilter { public Mono filter(ServerWebExchange exchange, WebFilterChain chain) { ServerHttpRequest request = exchange.getRequest(); HttpMethod httpMethod = request.getMethod(); - String[] urlParts = request.getPath().value().split("/"); + String url = request.getPath().value(); + String[] urlParts = url.split("/"); // This is because all the urls are of the form /api/v1/{resource}. When we split by "/", the resource is always // the 4th element in the result array @@ -55,7 +56,7 @@ public class AclFilter implements WebFilter { String resource = urlParts[3]; - Mono aclResponse = aclService.evaluateAcl(httpMethod, resource); + Mono aclResponse = aclService.evaluateAcl(httpMethod, resource, url); return aclResponse .map(acl -> { log.debug("Got ACL response: {}", acl); diff --git a/app/server/appsmith-server/src/main/resources/public/appsmith/authz/acl.rego b/app/server/appsmith-server/src/main/resources/public/appsmith/authz/acl.rego index 1a69acfcc1..e28ec299fc 100644 --- a/app/server/appsmith-server/src/main/resources/public/appsmith/authz/acl.rego +++ b/app/server/appsmith-server/src/main/resources/public/appsmith/authz/acl.rego @@ -5,18 +5,34 @@ default resource_allow = false # This rule allows the user to access endpoints based on the permissions that they are assigned. # The httpMethod and resource are also an integral part of accessing this ACL +# We overload the rule url_allow thereby making it an OR condition. Either the user is able to access the resource +# via their authenticated session, or it's a public url that is accessible to everybody. url_allow = true { - op = allowed_operations[_] + op = authenticated_operations[_] input.method = op.method input.resource = op.resource p = input.user.permissions[_] p = op.permission } +url_allow = true { + op = public_operations[_] + input.url = op.url + input.method = op.method +} + +# All public URLs must go into this list. Anything not in this list requires an authenticated session to access +public_operations = [ + {"method" : "POST", "url" : "/api/v1/users/forgotPassword" }, + {"method" : "POST", "url" : "/api/v1/users" }, + {"method" : "POST", "url" : "/api/v1/users/verifyPasswordResetToken" }, + {"method" : "POST", "url" : "/api/v1/users/resetPassword" }, +] + # This is a global list of all the routes for all controllers. Any new controller that is written must # carry an entry in this array. OPA performs ACL based on an intersection of these entries and permissions # for a user + permissions inherited via the groups that the user is a part of. -allowed_operations = [ +authenticated_operations = [ {"method": "POST", "resource": "users", "permission": "create:users"}, {"method": "GET", "resource": "users", "permission": "read:users"}, {"method": "PUT", "resource": "users", "permission": "update:users"}, diff --git a/app/server/appsmith-server/src/main/resources/public/bundle.tar.gz b/app/server/appsmith-server/src/main/resources/public/bundle.tar.gz index 930e45db7c711564080d3e79ccc513e9956a3fe4..68b8ba98b98bff48c507ae80df9e1a17c46d6772 100644 GIT binary patch literal 1261 zcmV-iIY#0_IfGzUyFR^EtDh z+401Fx`D3eL|gfqm~K_Pz8&Ms(>7bEf#@wXUFdIKQeF= ze_X58ho{Mc$oQYmvS~J+Wf1?#$??f^{Qrru7k`|yRFEbAre}aZW|N68{^Rj+1ReN!zw<9{Ci3g_?elAtCJQj-TsQji6%nL@Q-x0o@$L9fsPGf8$Uf=FJAoa`+p z-=V7&l}OYKsCr2vwIZmNL?Fcx&T}FqB4tr=S}KX|up|ZIrID(LSW_t}FZC|0^yC&{ z0VtSVmZV6{0R*j-s(vME#S4VXVyBIjWfBf55%9@UV1_Cz6yghaI4zgJ1s%_RyhyqR zDp3>kF+nhR1<%?Sq*OG=iWIjTl*3A^c71zeglJjSDqUMGUcnysy|cK}%PcofMnIon zg|u(dj_D1*vb)-q(s0V#(DUg16xUHhenzOu`pz8Ql{>7u)90KZqpiioYY}m#~E1 z-P8YOm*Vh;(US2y$S_8w%`nDHA`4zCA`#ecrgB~?!5IT&iq6U{D#@lZX>%QT8$p#K zC0fI4369CJ5L=zekcIGtmNsu;7H?B@IX}|{181yFF1MxRa$iW2>(FV%Kq*NKhk{I$5{#?b`m~#jGhCgeiLY{oIn;?jQyqHF1|KF{ zRrGW&2oPV|)%|F~`p@UFBmfnD&iv0JMfs5F60|vI%P#`ao<0%qGA+W{mAP50zT88fmo`=1dOZhW2`C z&e#C$_4&p5)p<0qAdK{ZF|KRA`<&OVN4hTnbPfz7hb{~dofCV+sA6IDl6n)SS&c0d zAFQd70PcYT@zI*;4ggTQS!w&5ZQl0U8)EE!IJ{*3ZbtNJ_ z5QI7pJcb>5AV_r{*rUefK)IkxfBfxgbnHE_UEKY+hhpxb?c(idiSF-ucNnIzdHc&; zUtZX=(VO2d&|ft%omBt&g2T%JIrCi zG(mD`GhJ)NOQz)+*Vh$XW7ei``|a|oZ8PnL6un(h(6zlAFsg4Th}q}i6%3qcn#o07 znncHps_l`Z_O5nX){3wSFpVEJ&>0exGYn*@$=WGfgLwTznhb7;+;zpl-nE0trDnEs zVd7e@MgqpQZC?+qWlD>_2rW8i?GF04 X@B5y|^LQT5qZxk#C~5^n03HAUrh82pi`icN zm#|+>(DbWD;jsUC{R^D@i8F%geGr>{5Jf3jz64GJzQ?uuzDv?cy}gfD18R{&XF6 z1u9Vz^eIAMynuT%PSQsXVPU0%x_^8yZkiXRinrDJuVJ?R-f=b&8FzhWCTMmwNoOVQ zh_3lX^;GMLJ~x?#4@Mc|Paup@SqWpzjZ0TO$yrz+LQ}P5}hp_7z8DW z;ZTT6SE{2HmNJo6o8$Xl^=ZB#5EX#`rr2o9KwQyoi|Qeh3|rnf8Ws%lcyBWLX;M{a z@(N9EZ{FTb&d{VTiXPK6sR!F2^l%~wR^(OX{d_h+{c;sb0#NGa+`bFtlttQ$C`cU_ z8P4g)Dkn~r{aImZkC2V(qufIw>*HP);jxAUTfnQ%6=dZL49)?vvAI#hDmzHF$_*(* z9m6{~{w{sBH&CGyW;Ob7LzPaN)!Cs1)lujZuBf}1?S#;ZdXRNl{m=uo^)TxK_n0Bt zapF5_wKq_;6J{m*a6`RLnicDzy}7!+y1NPnri76`FeG*LcUSUSe_;ANK$}26a+pF7 z(I&A+3_2{V%&2q2)ZWlz;)2y@#DhC`}w^K)$A#yZ)6oxb_ZMQ+GG+P|Y2* zrrwqoMGsNGX*Mm*`(JN$w_L4_-u`io{wj&-q5AzYhnEBNWIsb2MLytd!N^(L9BuIq zJ(0SN)jUby)^v@qB(!IOBwVDnsp2`)^BLFuG1r)_>DYe1xoaED>VX)&-%yCP?Ve5O z&lILr_rsnaRy1woy3EZ+$Be4onWgrAOK4de!V19Dd3c7Nkf4NNAWKcwN!c01n;+w7 z@RNjX=uh|Bb+ec1(N3RkTdwHTX6(L{54!R0+-h2>yGp%XP5U05`rCQqHaik9wr%@z xXencw_NUOavsSavxBh`?w9!TzZM4xw8*Q}FMjLIk(MJ2`_75WeM`!>j003>VPcZ-h