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

fix(auth): Rolling back group configuration to a map format #526

Merged
merged 3 commits into from
Dec 8, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ out/*
logs/*

### Kafka HQ ###
src/**/*-*.yml
src/main/*-*.yml
connects-plugins/

### Docs
Expand Down
13 changes: 12 additions & 1 deletion src/main/java/org/akhq/configs/SecurityProperties.java
Original file line number Diff line number Diff line change
@@ -1,15 +1,26 @@
package org.akhq.configs;

import io.micronaut.context.annotation.ConfigurationProperties;
import io.micronaut.core.convert.format.MapFormat;
import lombok.Data;

import javax.annotation.PostConstruct;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

@ConfigurationProperties("akhq.security")
@Data
public class SecurityProperties {
private List<BasicAuth> basicAuth = new ArrayList<>();
private List<Group> groups;
private String defaultGroup;

@MapFormat(transformation = MapFormat.MapTransformation.FLAT)
private Map<String, Group> groups = new HashMap<>();

@PostConstruct
public void init() {
groups.entrySet().forEach(entry -> entry.getValue().setName(entry.getKey()));
}
}
4 changes: 2 additions & 2 deletions src/main/java/org/akhq/utils/UserGroupUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public List<String> getUserRoles(List<String> groups) {
return new ArrayList<>();
}

return securityProperties.getGroups().stream()
return securityProperties.getGroups().values().stream()
.filter(group -> groups.contains(group.getName()))
.filter(group -> group.getRoles() != null)
.flatMap(group -> group.getRoles().stream())
Expand All @@ -48,7 +48,7 @@ public Map<String, Object> getUserAttributes(List<String> groups) {
return null;
}

return securityProperties.getGroups().stream()
return securityProperties.getGroups().values().stream()
.filter(group -> groups.contains(group.getName()))
.flatMap(group -> (group.getAttributes() != null) ? group.getAttributes().entrySet().stream() : null)
.collect(Collectors.toMap(
Expand Down
72 changes: 36 additions & 36 deletions src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -133,39 +133,39 @@ akhq:
security:
default-group: admin
groups:
- name: admin
roles:
- topic/read
- topic/insert
- topic/delete
- topic/config/update
- node/read
- node/config/update
- topic/data/read
- topic/data/insert
- topic/data/delete
- group/read
- group/delete
- group/offsets/update
- registry/read
- registry/insert
- registry/update
- registry/delete
- registry/version/delete
- acls/read
- connect/read
- connect/insert
- connect/update
- connect/delete
- connect/state/update
- name: reader
roles:
- topic/read
- node/read
- topic/data/read
- group/read
- registry/read
- acls/read
- connect/read
- name: no-roles
roles: []
admin:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor changes here :

admin: 
  name: Admin
   roles: 

And everywhere else.

We need to have a name & a key here because people with LDAP have group case sensitive.
I have a lot of issues about that in the past that lead me to go to array (that was not a good idea at the end).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a name property to the default groups with the same name as the key. I also changed the PostConstruct of SecurityProperties so that, when a name is not specified in the configuration, the key will be used as default.

roles:
- topic/read
- topic/insert
- topic/delete
- topic/config/update
- node/read
- node/config/update
- topic/data/read
- topic/data/insert
- topic/data/delete
- group/read
- group/delete
- group/offsets/update
- registry/read
- registry/insert
- registry/update
- registry/delete
- registry/version/delete
- acls/read
- connect/read
- connect/insert
- connect/update
- connect/delete
- connect/state/update
reader:
roles:
- topic/read
- node/read
- topic/data/read
- group/read
- registry/read
- acls/read
- connect/read
no-roles:
roles: []
56 changes: 56 additions & 0 deletions src/test/java/org/akhq/configs/SecurityPropertiesTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package org.akhq.configs;

import io.micronaut.context.ApplicationContext;
import org.junit.jupiter.api.Test;
import org.junit.platform.commons.util.CollectionUtils;

import java.util.Collections;

import static org.junit.jupiter.api.Assertions.assertEquals;

class SecurityPropertiesTest {

@Test
void shouldReturnAllBasicGroups() {
ApplicationContext ctx = ApplicationContext.run(ApplicationContext.class);
SecurityProperties securityProperties = ctx.getBean(SecurityProperties.class);

assertEquals(
CollectionUtils.toSet(new String[] {"admin", "limited", "operator", "no-filter"}),
securityProperties.getGroups().keySet()
);

ctx.close();
}

@Test
void shouldReturnAllBasicPlusConfiguredGroups() {
ApplicationContext ctx = ApplicationContext.run(ApplicationContext.class, "extragroups");
SecurityProperties securityProperties = ctx.getBean(SecurityProperties.class);

assertEquals(
CollectionUtils.toSet(new String[] {"admin", "limited", "operator", "no-filter", "extra", "another"}),
securityProperties.getGroups().keySet()
);

ctx.close();
}

@Test
void shouldOverrideBasicGroups() {
ApplicationContext ctx = ApplicationContext.run(ApplicationContext.class, "overridegroups");
SecurityProperties securityProperties = ctx.getBean(SecurityProperties.class);

assertEquals(
CollectionUtils.toSet(new String[] {"admin", "limited", "operator", "no-filter", "extra"}),
securityProperties.getGroups().keySet()
);
assertEquals(
Collections.singletonList("topic/read"),
securityProperties.getGroups().get("admin").roles
);

ctx.close();
}

}
10 changes: 10 additions & 0 deletions src/test/resources/application-extragroups.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
akhq:
security:
groups:
extra:
roles:
- topic/read
another:
roles:
- topic/read

13 changes: 13 additions & 0 deletions src/test/resources/application-overridegroups.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
akhq:
security:
groups:
admin:
roles:
- topic/read
limited:
roles:
- topic/read
extra:
roles:
- topic/read

8 changes: 4 additions & 4 deletions src/test/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ akhq:
security:
default-group: no-filter
groups:
- name: admin
admin:
roles:
- topic/read
- topic/insert
Expand All @@ -103,23 +103,23 @@ akhq:
- connect/update
- connect/delete
- connect/state/update
- name: limited
limited:
roles:
- topic/read
- topic/insert
- topic/delete
- registry/version/delete
attributes:
topics-filter-regexp: "test.*"
- name: operator
operator:
roles:
- topic/read
- topic/data/read
- topic/data/insert
- topic/data/delete
attributes:
topics-filter-regexp: "test-operator.*"
- name: no-filter
no-filter:
roles:
- topic/read
- topic/insert
Expand Down