trjhtr

I currently have a special service to obtain information about the logged in user. I have an interface

/**
 * The service operates on the data of an authorised user.
 */
public interface AuthorizationService 
 User getUser();
 String getUserId();
 String getUsername();
 boolean isLogged();

and implementation

@Service("authorizationService")
public class AuthorizationServiceImpl implements AuthorizationService 
 private final UserSearchService userSearchService;
 /**
 * @inheritDoc
 */
 @Override
 public User getUser() 
 return userSearchService.getUserByUsername(this.getUsername());
 

 /**
 * @inheritDoc
 */
 @Override
 public String getUserId() 
 return userSearchService.getUserByUsername(SecurityContextHolder.getContext().getAuthentication().getName()).getId();
 
...

and for example, when I use the method to create messages, I do so

@Override
public String createMessage(
 @NotNull @Valid final SendMessageDTO sendMessageDTO
) throws ResourceNotFoundException, ResourceConflictException 
 log.info("Called with sendMessageDTO ", sendMessageDTO);

 final UserEntity user = this.findUser(this.authorizationService.getUserId());
 final MessageEntity message = this.sendMessageDtoToMessageEntity(sendMessageDTO);

 message.setSender(user);

 ...

 private UserEntity findUser(final String id) throws ResourceNotFoundException 
 return this.userRepository
 .findByUniqueIdAndEnabledTrue(id)
 .orElseThrow(() -> new ResourceNotFoundException("No user found with id " + id));
 

1. However, I do not know if this is a good way to obtain information about the logged-in user?




2. Instead of a DTO User object, I could return an entity(I have a rule that I do not return entity service methods) and then I would not have to use the findUser () method to search for a user. What do you think about it?




3. Is my way of getting a DTO User logged in user or ID a safe way?

Usually the logged in user and it's common used information is stored in the session. And the session is usually wrapped in a Context class (like SecurityContextHolder itself). This is done because you can decouple your code from the session or from the servlet api (or whatever api you're using). It also makes it easier to test.

Most of web pages do show your user information when you're logged in, e.g. your avatar on this page on the top. So for every request you do not have to look up the user's information.

I'd introduce a Context class which provides a setUser and getUser method.

Beside that:

• 
  AuthorizationService: Authorization means: Is the user allowed to do x. (While authentication means: Is the user really the user he pretends to be). Or in other words: The name of the type is misleading.


• 
  getUser(): This method calls this.getUsername() -> I don't see the code of this one, bit I assume it looks up the username by SecurityContextHolder. This means, the AuthorizationService is tightly coupled Spring Security, which is bad. Same for getUserId() -> it's better to pass the arguments and put the responsibility to the presentation layer (which includes security usually)