Skip to content

Commit

Permalink
Merged AccountService into CredentialsService
Browse files Browse the repository at this point in the history
  • Loading branch information
Travis Tomsu committed Jun 6, 2016
1 parent 7c0ed2c commit 12707d4
Show file tree
Hide file tree
Showing 11 changed files with 78 additions and 167 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ interface ClouddriverService {
static class Account {
String name
String type
Collection<String> requiredGroupMembership
Collection<String> requiredGroupMembership = []
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,15 @@

package com.netflix.spinnaker.gate.controllers

import com.netflix.spinnaker.gate.security.SpinnakerUser
import com.netflix.spinnaker.gate.services.CredentialsService
import com.netflix.spinnaker.gate.services.internal.ClouddriverService
import com.netflix.spinnaker.security.User
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.web.bind.annotation.*
import org.springframework.web.bind.annotation.PathVariable
import org.springframework.web.bind.annotation.RequestMapping
import org.springframework.web.bind.annotation.RequestMethod
import org.springframework.web.bind.annotation.RestController

@RestController
@RequestMapping("/credentials")
Expand All @@ -29,8 +34,8 @@ class CredentialsController {
CredentialsService credentialsService

@RequestMapping(method = RequestMethod.GET)
List<ClouddriverService.Account> getAccounts() {
credentialsService.accounts
List<ClouddriverService.Account> getAccounts(@SpinnakerUser User user) {
credentialsService.getAccounts(user.roles)
}

@RequestMapping(value = '/{account}', method = RequestMethod.GET)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
package com.netflix.spinnaker.gate.security.anonymous

import com.netflix.spinnaker.gate.security.SpinnakerAuthConfig
import com.netflix.spinnaker.gate.services.AccountsService
import com.netflix.spinnaker.gate.services.CredentialsService
import com.netflix.spinnaker.security.User
import groovy.util.logging.Slf4j
import org.apache.commons.lang.exception.ExceptionUtils
Expand All @@ -40,7 +40,7 @@ class AnonymousConfig extends WebSecurityConfigurerAdapter {
static String defaultEmail = "anonymous"

@Autowired
AccountsService accountsService
CredentialsService credentialsService

List<String> anonymousAllowedAccounts = new CopyOnWriteArrayList<>()

Expand All @@ -60,7 +60,7 @@ class AnonymousConfig extends WebSecurityConfigurerAdapter {
@Scheduled(fixedDelay = 60000L)
void updateAnonymousAccounts() {
try {
def newAnonAccounts = accountsService.getAllowedAccounts([]) ?: []
def newAnonAccounts = credentialsService.getAccountNames([]) ?: []

def toAdd = newAnonAccounts - anonymousAllowedAccounts
def toRemove = anonymousAllowedAccounts - newAnonAccounts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package com.netflix.spinnaker.gate.security.oauth2
import com.netflix.spinnaker.gate.security.AuthConfig
import com.netflix.spinnaker.gate.security.SpinnakerAuthConfig
import com.netflix.spinnaker.gate.security.rolesprovider.UserRolesProvider
import com.netflix.spinnaker.gate.services.AccountsService
import com.netflix.spinnaker.gate.services.CredentialsService
import com.netflix.spinnaker.security.User
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression
Expand Down Expand Up @@ -94,7 +94,7 @@ class OAuth2SsoConfig extends OAuth2SsoConfigurerAdapter {
UserInfoTokenServices userInfoTokenServices

@Autowired
AccountsService accountsService
CredentialsService credentialsService

@Autowired
UserRolesProvider userRolesProvider
Expand All @@ -114,7 +114,7 @@ class OAuth2SsoConfig extends OAuth2SsoConfigurerAdapter {
email: details[userInfoMapping.email] as String,
firstName: details[userInfoMapping.firstName] as String,
lastName: details[userInfoMapping.lastName] as String,
allowedAccounts: accountsService.getAllowedAccounts(roles),
allowedAccounts: credentialsService.getAccountNames(roles),
roles: roles,
username: username).asImmutable()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package com.netflix.spinnaker.gate.security.saml
import com.netflix.spinnaker.gate.security.AuthConfig
import com.netflix.spinnaker.gate.security.SpinnakerAuthConfig
import com.netflix.spinnaker.gate.security.rolesprovider.UserRolesProvider
import com.netflix.spinnaker.gate.services.AccountsService
import com.netflix.spinnaker.gate.services.CredentialsService
import com.netflix.spinnaker.gate.services.internal.ClouddriverService
import com.netflix.spinnaker.security.User
import groovy.util.logging.Slf4j
Expand Down Expand Up @@ -136,7 +136,7 @@ class SamlSsoConfig extends WebSecurityConfigurerAdapter {
new SAMLUserDetailsService() {

@Autowired
AccountsService accountsService
CredentialsService credentialsService

@Autowired
ClouddriverService clouddriverService
Expand All @@ -157,7 +157,7 @@ class SamlSsoConfig extends WebSecurityConfigurerAdapter {
firstName: attributes[userAttributeMapping.firstName]?.get(0),
lastName: attributes[userAttributeMapping.lastName]?.get(0),
roles: roles,
allowedAccounts: accountsService.getAllowedAccounts(roles),
allowedAccounts: credentialsService.getAccountNames(roles),
username: attributes[userAttributeMapping.username] ?: email).asImmutable()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
package com.netflix.spinnaker.gate.security.x509

import com.netflix.spinnaker.gate.security.rolesprovider.UserRolesProvider
import com.netflix.spinnaker.gate.services.AccountsService
import com.netflix.spinnaker.gate.services.CredentialsService
import com.netflix.spinnaker.security.User
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.security.core.userdetails.AuthenticationUserDetailsService
Expand All @@ -39,7 +39,7 @@ class X509AuthenticationUserDetailsService implements AuthenticationUserDetailsS
private static final String RFC822_NAME_ID = "1"

@Autowired
AccountsService accountsService
CredentialsService credentialsService

@Autowired
UserRolesProvider userRolesProvider
Expand All @@ -56,7 +56,7 @@ class X509AuthenticationUserDetailsService implements AuthenticationUserDetailsS

return new User(
email: email,
allowedAccounts: accountsService.getAllowedAccounts(roles),
allowedAccounts: credentialsService.getAccountNames(roles),
roles: roles
).asImmutable()
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package com.netflix.spinnaker.gate.services

import com.netflix.spinnaker.gate.services.commands.HystrixFactory
import com.netflix.spinnaker.gate.services.internal.ClouddriverService
import com.netflix.spinnaker.security.AuthenticatedRequest
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.stereotype.Service

Expand All @@ -29,21 +28,33 @@ class CredentialsService {
@Autowired
ClouddriverService clouddriverService

List<ClouddriverService.Account> getAccounts() {
Collection<String> getAccountNames() {
getAccountNames([])
}

Collection<String> getAccountNames(Collection<String> userRoles) {
getAccounts(userRoles)*.name
}

Collection<ClouddriverService.Account> getAccounts() {
getAccounts([])
}

/**
* Returns all account names that a user with the specified list of userRoles has access to.
*/
List<ClouddriverService.Account> getAccounts(Collection<String> userRoles) {
HystrixFactory.newListCommand(GROUP, "getAccounts") {
def allAccounts = clouddriverService.accounts

if (!AuthenticatedRequest.getSpinnakerUser().present ||
!AuthenticatedRequest.getSpinnakerAccounts().present) {
// if the request is unauthenticated, return only anonymously accessible accounts (no group membership required)
return allAccounts.findAll { !it.requiredGroupMembership }
}

def allowedAccountsOptional = AuthenticatedRequest.getSpinnakerAccounts()
def allowedAccounts = allowedAccountsOptional.orElse("").split(",").collect { it.toLowerCase() }
return allAccounts.findAll {
allowedAccounts.contains(it.name.toLowerCase())
}
return clouddriverService.accounts.findAll { ClouddriverService.Account account ->
if (!account.requiredGroupMembership) {
return true // anonymous account.
}

def userRolesLower = userRoles*.toLowerCase()
def reqGroupMembershipLower = account.requiredGroupMembership*.toLowerCase()

return userRolesLower.intersect(reqGroupMembershipLower) as Boolean
} ?: []
} execute()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

package com.netflix.spinnaker.gate.security.anonymous

import com.netflix.spinnaker.gate.services.AccountsService
import com.netflix.spinnaker.gate.services.CredentialsService
import spock.lang.Specification
import spock.lang.Subject
import spock.lang.Unroll
Expand All @@ -28,16 +28,15 @@ class AnonymousConfigSpec extends Specification {
@Unroll
def "should update accounts correctly"() {
setup:
AccountsService accountsService = Mock(AccountsService) {
getAllowedAccounts(*_) >> newAccounts
CredentialsService credentialsService = Mock(CredentialsService) {
getAccountNames(*_) >> newAccounts
}
@Subject
AnonymousConfig config = new AnonymousConfig(
anonymousAllowedAccounts: new CopyOnWriteArrayList<String>(oldAccounts),
accountsService: accountsService
credentialsService: credentialsService
)


when:
config.updateAnonymousAccounts()

Expand Down

This file was deleted.

Loading

0 comments on commit 12707d4

Please sign in to comment.