Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance authorization and authentication in save-cloud #2336

Open
5 of 9 tasks
kgevorkyan opened this issue Jul 14, 2023 · 9 comments
Open
5 of 9 tasks

Enhance authorization and authentication in save-cloud #2336

kgevorkyan opened this issue Jul 14, 2023 · 9 comments
Assignees
Labels
auth Issues related to authentication, authorization and overall service security help wanted Extra attention is needed vulnerabilities

Comments

@kgevorkyan
Copy link
Member

kgevorkyan commented Jul 14, 2023

TODO:

Separator for user and auth provider should be revised

For now we are using @ for separate the user name and auth provider at least in WebSecurityConfig and in UserUtils.kt

But this symbol is not the variable and just the hardcoded symbol, so its hard to maintain this functionality

First of all, it should be extracted into the variable in save-cloud-common

And after this, we probably will need to change @ to something else, since @ is too dangerous to be a spec symbol

This work should be done under separate issue and very carefully, since it influence on core functionality - API application, auth. integration tests, db

@kgevorkyan kgevorkyan added help wanted Extra attention is needed auth Issues related to authentication, authorization and overall service security vulnerabilities labels Jul 14, 2023
@orchestr7
Copy link
Member

orchestr7 commented Jul 14, 2023

CRITICAL thing we have just found:

  1. User VLAD comes from Github
  2. Initially spring register him as VLAD@GITHUB
  3. Then he is redirected to a registration view, where he CAN change VLAD name to ANDREY
  4. ANDREY comes from GITHUB
  5. Now ANDREY is not even able to press OAuth registration button

@orchestr7
Copy link
Member

orchestr7 commented Jul 14, 2023

That's why we need to GENERATE a our own SAVE ID for user when he first time register!

orchestr7 added a commit that referenced this issue Jul 14, 2023
### What's done:
- SAVE uses '@' as a special mark to separate user and source #2336 so we cannot use e-mail as login in SAVE by for now
orchestr7 added a commit that referenced this issue Jul 17, 2023
### What's done:
- SAVE uses '@' as a special mark to separate user and source #2336 so we cannot use e-mail as login in SAVE by for now
@kgevorkyan
Copy link
Member Author

seems, that @ should not influent on problems with email at all, since its only used in basic auth, but should be extracted anyway

nulls added a commit that referenced this issue Jul 17, 2023
### What's done:
- removed headers
- removed source in internal communication

It closes #2336
nulls added a commit that referenced this issue Jul 17, 2023
### What's done:
- removed headers
- removed source in internal communication

It closes #2336
@Cheshiriks
Copy link
Member

        .httpBasic { httpBasicSpec ->
            // Authenticate by comparing received basic credentials with existing one from DB
            httpBasicSpec.authenticationManager(
                UserDetailsRepositoryReactiveAuthenticationManager { username ->
                    // Looking for user in DB by received source and name
                    require(username.contains("@")) {
                        "Provided user information should keep the following form: oauth2Source@username"
                    }

Also be careful with Basic oauth

@Cheshiriks
Copy link
Member

That's why we need to GENERATE a our own SAVE ID for user when he first time register!

    fun findByName(name: String): User? {
        val record = namedParameterJdbcTemplate.queryForList(
            "SELECT * FROM save_cloud.user WHERE name = :name",
            mapOf("name" to name)
        ).singleOrNull() ?:
        namedParameterJdbcTemplate.queryForList(
            "SELECT * FROM save_cloud.user WHERE id = (select user_id from save_cloud.original_login where name = :name)",
            mapOf("name" to name)
        ).singleOrNull()
            .orNotFound {
                "There is no user with name $name"
            }
        return record.toUserEntity()
    }

nulls added a commit that referenced this issue Jul 20, 2023
### What's done:
- small refactoring to hide AuthenticationDetails to get userId and identitySource

It's part of #2336
nulls added a commit that referenced this issue Jul 20, 2023
### What's done:
- small refactoring to mark name and source are not nullable in database and entity

It's part of #2336
nulls added a commit that referenced this issue Jul 20, 2023
### What's done:
- small refactoring to hide AuthenticationDetails to get userId and identitySource

It's part of #2336
nulls added a commit that referenced this issue Jul 20, 2023
nulls added a commit that referenced this issue Jul 20, 2023
### What's done:
- a small refactoring

It's part of #2336
@nulls
Copy link
Member

nulls commented Jul 21, 2023

For history

A&A in save-cloud:

Basic auth:

  1. Request comes to api-gateway with header Authorization: Basic ***:***
  2. api-gateway has UserDetailsRepositoryReactiveAuthenticationManager (com.saveourtool.save.gateway.security.WebSecurityConfig) which uses internal endpoint in backend to get all entities from user with user.name, user.password (token) and user.role.
  3. Authenticated request comes then to com.saveourtool.save.gateway.utils.ConvertAuthorizationHeaderGatewayFilterFactory which adds a new header Authorization to request to backend. It passes only user name without password.
  4. Backend uses com.saveourtool.save.authservice.security.ConvertingAuthenticationManager to authenticate: it checks only username

OAUTH2

  1. Request comes to api-gateway after successful authentication on oauth2 server (github, google and etc).
  2. Authenticated request comes then to com.saveourtool.save.gateway.utils.ConvertAuthorizationHeaderGatewayFilterFactory which searches user.name via internal endpoint in backend and adds a new header Authorization to request to backend. It passes only user name without password.
  3. Backend uses com.saveourtool.save.authservice.security.ConvertingAuthenticationManager to authenticate: it checks only username

@kgevorkyan
Copy link
Member Author

For history

A&A in save-cloud:

Basic auth:

  1. Request comes to api-gateway with header Authorization: Basic ***:***
  2. api-gateway has UserDetailsRepositoryReactiveAuthenticationManager (com.saveourtool.save.gateway.security.WebSecurityConfig) which uses internal endpoint in backend to get all entities from user with user.name, user.password (token) and user.role.
  3. Authenticated request comes then to com.saveourtool.save.gateway.utils.ConvertAuthorizationHeaderGatewayFilterFactory which adds a new header Authorization to request to backend. It passes only user name without password.
  4. Backend uses com.saveourtool.save.authservice.security.ConvertingAuthenticationManager to authenticate: it checks only username

OAUTH2

  1. Request comes to api-gateway after successful authentication on oauth2 server (github, google and etc).
  2. Authenticated request comes then to com.saveourtool.save.gateway.utils.ConvertAuthorizationHeaderGatewayFilterFactory which searches user.name via internal endpoint in backend and adds a new header Authorization to request to backend. It passes only user name without password.
  3. Backend uses com.saveourtool.save.authservice.security.ConvertingAuthenticationManager to authenticate: it checks only username

may be add in readme, into some dev guide?

@nulls
Copy link
Member

nulls commented Jul 21, 2023

For history

A&A in save-cloud:

Basic auth:

  1. Request comes to api-gateway with header Authorization: Basic ***:***
  2. api-gateway has UserDetailsRepositoryReactiveAuthenticationManager (com.saveourtool.save.gateway.security.WebSecurityConfig) which uses internal endpoint in backend to get all entities from user with user.name, user.password (token) and user.role.
  3. Authenticated request comes then to com.saveourtool.save.gateway.utils.ConvertAuthorizationHeaderGatewayFilterFactory which adds a new header Authorization to request to backend. It passes only user name without password.
  4. Backend uses com.saveourtool.save.authservice.security.ConvertingAuthenticationManager to authenticate: it checks only username

OAUTH2

  1. Request comes to api-gateway after successful authentication on oauth2 server (github, google and etc).
  2. Authenticated request comes then to com.saveourtool.save.gateway.utils.ConvertAuthorizationHeaderGatewayFilterFactory which searches user.name via internal endpoint in backend and adds a new header Authorization to request to backend. It passes only user name without password.
  3. Backend uses com.saveourtool.save.authservice.security.ConvertingAuthenticationManager to authenticate: it checks only username

may be add in readme, into some dev guide?

will do, when finish with this issue

@nulls

This comment was marked as duplicate.

nulls added a commit that referenced this issue Jul 24, 2023
### What's done:
- avoid changing Spring's ObjectMapper

It's part of #2336
@nulls nulls pinned this issue Jul 24, 2023
@nulls nulls changed the title Separator for user and auth provider should be revised Enhance authorization and authentication in save-cloud Jul 24, 2023
@orchestr7 orchestr7 unpinned this issue Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth Issues related to authentication, authorization and overall service security help wanted Extra attention is needed vulnerabilities
Projects
None yet
Development

No branches or pull requests

4 participants