From a34350b62784d4332243ba40ffe928afd91f67d3 Mon Sep 17 00:00:00 2001 From: Vitaly Takmazov Date: Fri, 26 Apr 2019 16:40:58 +0300 Subject: Authorization checks are in spring-security for a while --- .../java/com/juick/server/api/ApiSocialLogin.java | 20 +++++++--------- .../java/com/juick/server/api/Notifications.java | 28 +++++++++------------- src/main/java/com/juick/server/api/PM.java | 9 ------- src/main/java/com/juick/server/api/Post.java | 28 +++++----------------- src/main/java/com/juick/server/api/Service.java | 2 +- .../juick/server/www/controllers/MessagesWWW.java | 5 +--- .../java/com/juick/server/tests/ServerTests.java | 9 +++++++ 7 files changed, 37 insertions(+), 64 deletions(-) diff --git a/src/main/java/com/juick/server/api/ApiSocialLogin.java b/src/main/java/com/juick/server/api/ApiSocialLogin.java index efc2e288..fe5f2069 100644 --- a/src/main/java/com/juick/server/api/ApiSocialLogin.java +++ b/src/main/java/com/juick/server/api/ApiSocialLogin.java @@ -24,7 +24,6 @@ import com.github.scribejava.core.model.OAuth2AccessToken; import com.github.scribejava.core.model.OAuthRequest; import com.github.scribejava.core.model.Verb; import com.github.scribejava.core.oauth.OAuth20Service; -import com.google.api.client.auth.openidconnect.IdToken; import com.google.api.client.googleapis.auth.oauth2.GoogleIdToken; import com.google.api.client.googleapis.auth.oauth2.GoogleIdTokenVerifier; import com.google.api.client.http.HttpTransport; @@ -34,7 +33,6 @@ import com.google.api.client.json.jackson2.JacksonFactory; import com.juick.model.Auth; import com.juick.model.facebook.User; import com.juick.server.util.HttpBadRequestException; -import com.juick.server.util.HttpForbiddenException; import com.juick.service.CrosspostService; import com.juick.service.EmailService; import com.juick.service.TelegramService; @@ -46,6 +44,8 @@ import org.apache.commons.lang3.math.NumberUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Value; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PostMapping; @@ -57,9 +57,7 @@ import javax.annotation.PostConstruct; import javax.inject.Inject; import java.io.IOException; import java.security.GeneralSecurityException; -import java.util.Arrays; import java.util.Collections; -import java.util.List; import java.util.UUID; import java.util.concurrent.ExecutionException; @@ -172,7 +170,7 @@ public class ApiSocialLogin { } else { if (!crosspostService.createFacebookUser(fbID, state, token.getAccessToken(), fb.getName())) { if (StringUtils.isNotEmpty(fb.getEmail())) { - logger.info("found {} for facebook user {}", fb.getEmail()); + logger.info("found {} for facebook user {}", fb.getEmail(), fb.getName()); Integer userId = crosspostService.getUIDbyFBID(fbID); if (!emailService.getEmails(userId, false).contains(fb.getEmail())) { emailService.addEmail(userId, fb.getEmail()); @@ -279,7 +277,7 @@ public class ApiSocialLogin { } @ResponseBody @PostMapping("/api/_google") - public Auth googleSignIn(@RequestParam(name = "idToken") String idTokenString) + public ResponseEntity googleSignIn(@RequestParam(name = "idToken") String idTokenString) throws GeneralSecurityException, IOException { logger.info("Token: {}", idTokenString); logger.info("Client: {}", googleClientId); @@ -289,14 +287,14 @@ public class ApiSocialLogin { if (userService.getUserByEmail(email).isAnonymous()) { String verificationCode = RandomStringUtils.randomAlphanumeric(8).toUpperCase(); emailService.addVerificationCode(null, email, verificationCode); - return new Auth(email, verificationCode); + return ResponseEntity.ok(new Auth(email, verificationCode)); } } - throw new HttpForbiddenException(); + return ResponseEntity.status(HttpStatus.FORBIDDEN).body(null); } @ResponseBody @PostMapping("/api/signup") - public com.juick.User signupWithEmail(String username, String password, String verificationCode) { + public ResponseEntity signupWithEmail(String username, String password, String verificationCode) { if (username.length() < 2 || username.length() > 16 || !username.matches("^[a-zA-Z0-9\\-]+$") || password.length() < 6 || password.length() > 32) { throw new HttpBadRequestException(); @@ -310,9 +308,9 @@ public class ApiSocialLogin { } emailService.addEmail(uid, verifiedEmail); emailService.deleteAuthCode(verificationCode); - return userService.getUserByUID(uid).orElseThrow(IllegalStateException::new); + return ResponseEntity.ok(userService.getUserByUID(uid).orElseThrow(IllegalStateException::new)); } else { - throw new HttpForbiddenException(); + return ResponseEntity.status(HttpStatus.FORBIDDEN).body(null); } } /* diff --git a/src/main/java/com/juick/server/api/Notifications.java b/src/main/java/com/juick/server/api/Notifications.java index ea1d5c54..6829653c 100644 --- a/src/main/java/com/juick/server/api/Notifications.java +++ b/src/main/java/com/juick/server/api/Notifications.java @@ -23,13 +23,13 @@ import com.juick.ExternalToken; import com.juick.User; import com.juick.model.AnonymousUser; import com.juick.server.util.HttpBadRequestException; -import com.juick.server.util.HttpForbiddenException; import com.juick.server.util.UserUtils; import com.juick.service.MessagesService; import com.juick.service.PushQueriesService; import com.juick.service.SubscriptionService; import com.juick.service.TelegramService; import com.juick.service.UserService; +import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.RequestBody; @@ -88,8 +88,8 @@ public class Notifications { @RequestParam(required = false, defaultValue = "0") int mid, @RequestParam(required = false, defaultValue = "0") int rid) { User visitor = UserUtils.getCurrentUser(); - if (visitor.isAnonymous() || !(visitor.getName().equals("juick"))) { - throw new HttpForbiddenException(); + if (!(visitor.getName().equals("juick"))) { + return ResponseEntity.status(HttpStatus.FORBIDDEN).body(null); } if (uid > 0 && mid == 0) { // PM @@ -118,11 +118,11 @@ public class Notifications { @ApiIgnore @RequestMapping(value = "/api/notifications", method = RequestMethod.DELETE, produces = MediaType.APPLICATION_JSON_UTF8_VALUE) - public Status doDelete( + public ResponseEntity doDelete( @RequestBody List list) { User visitor = UserUtils.getCurrentUser(); - if ((visitor.isAnonymous()) || !(visitor.getName().equals("juick"))) { - throw new HttpForbiddenException(); + if (!visitor.getName().equals("juick")) { + return ResponseEntity.status(HttpStatus.FORBIDDEN).body(null); } list.forEach(t -> { switch (t.getType()) { @@ -140,15 +140,15 @@ public class Notifications { } }); - return Status.OK; + return ResponseEntity.ok(Status.OK); } @ApiIgnore @RequestMapping(value = "/api/notifications/delete", method = RequestMethod.POST, produces = MediaType.APPLICATION_JSON_UTF8_VALUE) - public Status doDeleteTokens( + public ResponseEntity doDeleteTokens( @RequestBody List list) { User visitor = UserUtils.getCurrentUser(); - if ((visitor.isAnonymous()) || !(visitor.getName().equals("juick"))) { - throw new HttpForbiddenException(); + if (!visitor.getName().equals("juick")) { + return ResponseEntity.status(HttpStatus.FORBIDDEN).body(null); } list.forEach(t -> { switch (t.getType()) { @@ -166,7 +166,7 @@ public class Notifications { } }); - return Status.OK; + return ResponseEntity.ok(Status.OK); } @ApiIgnore @@ -174,9 +174,6 @@ public class Notifications { public Status doPut( @RequestBody List list) throws IOException { User visitor = UserUtils.getCurrentUser(); - if (visitor.isAnonymous()) { - throw new HttpForbiddenException(); - } list.forEach(t -> { switch (t.getType()) { case "gcm": @@ -200,9 +197,6 @@ public class Notifications { public Status doAndroidRegister( @RequestParam(name = "regid") String regId) { User visitor = UserUtils.getCurrentUser(); - if (visitor.isAnonymous()) { - throw new HttpForbiddenException(); - } pushQueriesService.addGCMToken(visitor.getUid(), regId); return Status.OK; } diff --git a/src/main/java/com/juick/server/api/PM.java b/src/main/java/com/juick/server/api/PM.java index e61fef6e..06dc9733 100644 --- a/src/main/java/com/juick/server/api/PM.java +++ b/src/main/java/com/juick/server/api/PM.java @@ -56,9 +56,6 @@ public class PM { public List doGetPM( @RequestParam(required = false) String uname) { User visitor = UserUtils.getCurrentUser(); - if (visitor.isAnonymous()) { - throw new HttpForbiddenException(); - } int uid = 0; if (uname != null && uname.matches("^[a-zA-Z0-9\\-]{2,16}$")) { uid = userService.getUIDbyName(uname); @@ -78,9 +75,6 @@ public class PM { @RequestParam String uname, @RequestParam String body) { User visitor = UserUtils.getCurrentUser(); - if (visitor.isAnonymous()) { - throw new HttpForbiddenException(); - } User userTo = AnonymousUser.INSTANCE; if (WebUtils.isUserName(uname)) { userTo = userService.getUserByName(uname); @@ -110,9 +104,6 @@ public class PM { public PrivateChats doGetGroupsPMs( @RequestParam(defaultValue = "5") int cnt) { User visitor = UserUtils.getCurrentUser(); - if (visitor.isAnonymous()) { - throw new HttpForbiddenException(); - } // TODO: ignore cnt param for now but make sure paging param will not be cnt List lastconv = pmQueriesService.getLastChats(visitor); diff --git a/src/main/java/com/juick/server/api/Post.java b/src/main/java/com/juick/server/api/Post.java index d49ec332..b575cef8 100644 --- a/src/main/java/com/juick/server/api/Post.java +++ b/src/main/java/com/juick/server/api/Post.java @@ -70,10 +70,6 @@ public class Post { @RequestParam(required = false) String img, @RequestParam(required = false) MultipartFile attach) throws Exception { User visitor = UserUtils.getCurrentUser(); - - if (visitor.isAnonymous()) { - throw new HttpForbiddenException(); - } body = body.replace("\r", StringUtils.EMPTY); URI attachmentFName = HttpUtils.receiveMultiPartFile(attach, tmpDir); @@ -103,20 +99,16 @@ public class Post { public CommandResult doPostComment( @RequestParam(defaultValue = "0") int mid, @RequestParam(defaultValue = "0") int rid, - @RequestParam(required = false, defaultValue = StringUtils.EMPTY) String body, + @RequestParam(required = false, defaultValue = StringUtils.EMPTY) final String body, @RequestParam(required = false) String img, @RequestParam(required = false) MultipartFile attach) throws Exception { User visitor = UserUtils.getCurrentUser(); - int vuid = visitor.getUid(); - if (vuid == 0) { - throw new HttpForbiddenException(); - } if (mid == 0) { throw new HttpBadRequestException(); } Optional message = messagesService.getMessage(mid); - if (!message.isPresent()) { + if (message.isEmpty()) { throw new HttpNotFoundException(); } @@ -129,10 +121,11 @@ public class Post { throw new HttpNotFoundException(); } } - body = body.replace("\r", StringUtils.EMPTY); - if ((msg.ReadOnly && msg.getUser().getUid() != vuid) || userService.isInBLAny(msg.getUser().getUid(), vuid) - || (reply != null && userService.isInBLAny(reply.getUser().getUid(), vuid))) { + if ((msg.ReadOnly && msg.getUser().getUid() != visitor.getUid()) + || userService.isInBLAny(msg.getUser().getUid(), visitor.getUid()) + || (reply != null && userService.isInBLAny(reply.getUser().getUid(), visitor.getUid()))) { + // TODO: validator throw new HttpForbiddenException(); } @@ -158,9 +151,6 @@ public class Post { @ResponseStatus(value = HttpStatus.OK) public Status doPostRecomm(@RequestParam Integer mid) throws Exception { com.juick.User visitor = UserUtils.getCurrentUser(); - if (visitor.isAnonymous()) { - throw new HttpForbiddenException(); - } Optional message = messagesService.getMessage(mid); if (!message.isPresent()) { throw new HttpNotFoundException(); @@ -178,9 +168,6 @@ public class Post { @ResponseStatus(value = HttpStatus.OK) public Status doPostSubscribe(@RequestParam Integer mid) throws Exception { com.juick.User visitor = UserUtils.getCurrentUser(); - if (visitor.isAnonymous()) { - throw new HttpForbiddenException(); - } Optional message = messagesService.getMessage(mid); if (!message.isPresent()) { throw new HttpNotFoundException(); @@ -207,9 +194,6 @@ public class Post { logger.info("got reaction with type: {}", reactionId); com.juick.User visitor = UserUtils.getCurrentUser(); - if (visitor.isAnonymous()) { - throw new HttpForbiddenException(); - } Optional message = messagesService.getMessage(mid); if (!message.isPresent()) { throw new HttpNotFoundException(); diff --git a/src/main/java/com/juick/server/api/Service.java b/src/main/java/com/juick/server/api/Service.java index ed62886f..27316d91 100644 --- a/src/main/java/com/juick/server/api/Service.java +++ b/src/main/java/com/juick/server/api/Service.java @@ -112,7 +112,7 @@ public class Service { IOUtils.copy(a.getInputStream(), fos); fos.close(); } catch (IOException e) { - logger.info("attachment error: {}", e); + logger.info("attachment error", e); } }); String[] inReplyToHeaders = msg.getHeader("In-Reply-To"); diff --git a/src/main/java/com/juick/server/www/controllers/MessagesWWW.java b/src/main/java/com/juick/server/www/controllers/MessagesWWW.java index aa79ee0e..41c95dcb 100644 --- a/src/main/java/com/juick/server/www/controllers/MessagesWWW.java +++ b/src/main/java/com/juick/server/www/controllers/MessagesWWW.java @@ -356,9 +356,6 @@ public class MessagesWWW { protected String doGetReaders(@PathVariable String uname, ModelMap model) throws IOException { com.juick.User user = userService.getUserByName(uname); com.juick.User visitor = UserUtils.getCurrentUser(); - if (visitor.isBanned()) { - throw new HttpForbiddenException(); - } visitor.setAvatar(webApp.getAvatarWebPath(visitor)); model.addAttribute("title", "Читатели " + user.getName()); model.addAttribute("headers", ""); @@ -373,7 +370,7 @@ public class MessagesWWW { protected String doGetBL(@PathVariable String uname, ModelMap model) throws IOException { com.juick.User user = userService.getUserByName(uname); com.juick.User visitor = UserUtils.getCurrentUser(); - if (visitor.isBanned() || visitor.getUid() != user.getUid()) { + if (visitor.getUid() != user.getUid()) { throw new HttpForbiddenException(); } visitor.setAvatar(webApp.getAvatarWebPath(visitor)); diff --git a/src/test/java/com/juick/server/tests/ServerTests.java b/src/test/java/com/juick/server/tests/ServerTests.java index 32e9929e..770c7e7c 100644 --- a/src/test/java/com/juick/server/tests/ServerTests.java +++ b/src/test/java/com/juick/server/tests/ServerTests.java @@ -2043,4 +2043,13 @@ public class ServerTests { Pair replyId = messagesService.findMessageByProperty("tg_id", "hrhr").orElseThrow(); assertThat(replyId.getRight(), is(rid)); } + @Test + public void forbiddenForAnonymousEndpoints() throws Exception { + mockMvc.perform(post("/api/comment")).andExpect(status().isUnauthorized()); + mockMvc.perform(post("/api/like")).andExpect(status().isUnauthorized()); + mockMvc.perform(post("/api/subscribe")).andExpect(status().isUnauthorized()); + mockMvc.perform(post("/api/react")).andExpect(status().isUnauthorized()); + mockMvc.perform(get("/api/notifications")).andExpect(status().isUnauthorized()); + mockMvc.perform(delete("/api/notifications")).andExpect(status().isUnauthorized()); + } } -- cgit v1.2.3