Skip to content

Commit 5ac61b1

Browse files
cmangeatcmangeat
committed
encrypt hash in db
choosen encryption algorithm forbid comparaison of same hash encrypted twice, have to first fecth stored crypted hash corresponding to uuid, then to check for match.
1 parent 0e89dc8 commit 5ac61b1

File tree

7 files changed

+53
-37
lines changed

7 files changed

+53
-37
lines changed

‎core/src/main/java/org/fao/geonet/security/GrantViewMdAuthorityFilter.java‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,16 @@
2626
import org.fao.geonet.domain.AnonymousAccessLink;
2727
import org.fao.geonet.kernel.security.ViewMdGrantedAuthority;
2828
import org.fao.geonet.repository.AnonymousAccessLinkRepository;
29+
import org.fao.geonet.util.PasswordUtil;
2930
import org.springframework.beans.factory.annotation.Autowired;
31+
import org.springframework.beans.factory.annotation.Qualifier;
3032
import org.springframework.security.authentication.AnonymousAuthenticationToken;
3133
import org.springframework.security.authentication.AuthenticationTrustResolver;
3234
import org.springframework.security.authentication.AuthenticationTrustResolverImpl;
3335
import org.springframework.security.core.Authentication;
3436
import org.springframework.security.core.GrantedAuthority;
3537
import org.springframework.security.core.context.SecurityContextHolder;
38+
import org.springframework.security.crypto.password.PasswordEncoder;
3639
import org.springframework.security.web.context.HttpSessionSecurityContextRepository;
3740
import org.springframework.web.filter.GenericFilterBean;
3841

@@ -49,6 +52,10 @@ public class GrantViewMdAuthorityFilter extends GenericFilterBean {
4952
@Autowired
5053
AnonymousAccessLinkRepository anonymousAccessLinkRepository;
5154

55+
@Autowired
56+
@Qualifier(PasswordUtil.ENCODER_ID)
57+
PasswordEncoder encoder;
58+
5259
private HttpSessionSecurityContextRepository repo;
5360

5461
public GrantViewMdAuthorityFilter(HttpSessionSecurityContextRepository httpSessionSecurityContextRepository) {
@@ -78,9 +85,15 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
7885
filterChain.doFilter(servletRequest, servletResponse);
7986
return;
8087
}
81-
String hash = servletRequest.getParameter("hash");
82-
AnonymousAccessLink authority = anonymousAccessLinkRepository.findOneByHash(hash);
83-
if (authority == null) {
88+
String hashParam = servletRequest.getParameter("hash");
89+
if (hashParam == null || hashParam.length() < AnonymousAccessLink.getRandomHashLength() + 1) {
90+
filterChain.doFilter(servletRequest, servletResponse);
91+
return;
92+
}
93+
String hash = hashParam.substring(0, AnonymousAccessLink.getRandomHashLength());
94+
String uuid = hashParam.substring(AnonymousAccessLink.getRandomHashLength());
95+
AnonymousAccessLink authority = anonymousAccessLinkRepository.findOneByMetadataUuid(uuid);
96+
if (authority == null || !encoder.matches(hash, authority.getHash())) {
8497
filterChain.doFilter(servletRequest, servletResponse);
8598
return;
8699
}

‎core/src/test/java/org/fao/geonet/security/GrantViewAuthorityFilterTest.java‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.springframework.mock.web.MockHttpServletRequest;
3434
import org.springframework.mock.web.MockHttpServletResponse;
3535
import org.springframework.security.core.context.SecurityContextHolder;
36+
import org.springframework.security.crypto.password.PasswordEncoder;
3637
import org.springframework.security.web.context.HttpSessionSecurityContextRepository;
3738

3839
import javax.servlet.ServletException;
@@ -45,10 +46,12 @@
4546
import static org.junit.Assert.assertTrue;
4647
import static org.mockito.ArgumentMatchers.argThat;
4748
import static org.mockito.Mockito.mock;
49+
import static org.mockito.Mockito.when;
4850

4951
public class GrantViewAuthorityFilterTest extends AbstractCoreIntegrationTest {
5052

5153
private int mdId;
54+
private String uuid;
5255
private String hash;
5356
private MockHttpServletRequest requestMock;
5457
private MockFilterChain filterChainMock;
@@ -105,9 +108,9 @@ public void canViewManyDifferentMds() throws ServletException, IOException {
105108

106109
toTest.doFilter(requestMock, responseMock, filterChainMock);
107110
filterChainMock.reset();
108-
Mockito.when(repositoryMock.findOneByHash(argThat("hush-hush"::equals))).thenReturn(
109-
new AnonymousAccessLink().setMetadataId(123));
110-
requestMock.setParameter("hash", "hush-hush");
111+
Mockito.when(repositoryMock.findOneByMetadataUuid(argThat("uuid2"::equals))).thenReturn(
112+
new AnonymousAccessLink().setMetadataId(123).setMetadataUuid("uuid2").setHash("Y".repeat(AnonymousAccessLink.getRandomHashLength())));
113+
requestMock.setParameter("hash", "Y".repeat(AnonymousAccessLink.getRandomHashLength()) + "uuid2");
111114
toTest.doFilter(requestMock, responseMock, filterChainMock);
112115

113116
assertEquals(requestMock, filterChainMock.getRequest());
@@ -127,7 +130,7 @@ public void canViewManyDifferentMds() throws ServletException, IOException {
127130
public void unknownHashIgnored() throws ServletException, IOException {
128131
GrantViewMdAuthorityFilter toTest = prepareToTest();
129132

130-
requestMock.setParameter("hash", "hush-hush");
133+
requestMock.setParameter("hash", "Y".repeat(AnonymousAccessLink.getRandomHashLength()) + uuid);
131134
toTest.doFilter(requestMock, responseMock, filterChainMock);
132135

133136
assertEquals(requestMock, filterChainMock.getRequest());
@@ -138,17 +141,26 @@ public void unknownHashIgnored() throws ServletException, IOException {
138141
}
139142

140143
private GrantViewMdAuthorityFilter prepareToTest() {
141-
hash = "hash-hash";
144+
uuid = "uuid";
145+
hash = "X".repeat(AnonymousAccessLink.getRandomHashLength()) + uuid;
142146
mdId = 666;
143147
repositoryMock = mock(AnonymousAccessLinkRepository.class);
144-
Mockito.when(repositoryMock.findOneByHash(argThat(hash::equals))).thenReturn(new AnonymousAccessLink().setMetadataId(mdId));
148+
Mockito.when(repositoryMock.findOneByMetadataUuid(argThat(uuid::equals))).thenReturn(
149+
new AnonymousAccessLink().setMetadataId(mdId).setMetadataUuid(uuid).setHash("X".repeat(AnonymousAccessLink.getRandomHashLength())));
150+
PasswordEncoder encodeMock = Mockito.mock(PasswordEncoder.class);
151+
when(encodeMock.matches("X".repeat(AnonymousAccessLink.getRandomHashLength()), "X".repeat(AnonymousAccessLink.getRandomHashLength()))).thenReturn(true);
152+
when(encodeMock.encode("X".repeat(AnonymousAccessLink.getRandomHashLength()))).thenReturn("X".repeat(AnonymousAccessLink.getRandomHashLength()));
153+
when(encodeMock.matches("Y".repeat(AnonymousAccessLink.getRandomHashLength()), "Y".repeat(AnonymousAccessLink.getRandomHashLength()))).thenReturn(true);
154+
when(encodeMock.encode("Y".repeat(AnonymousAccessLink.getRandomHashLength()))).thenReturn("Y".repeat(AnonymousAccessLink.getRandomHashLength()));
145155
GrantViewMdAuthorityFilter toTest = new GrantViewMdAuthorityFilter(mock(HttpSessionSecurityContextRepository.class));
146156
toTest.anonymousAccessLinkRepository = repositoryMock;
157+
toTest.encoder = encodeMock;
147158
requestMock = new MockHttpServletRequest();
148159
requestMock.setParameter("hash", hash);
149160
responseMock = new MockHttpServletResponse();
150161
filterChainMock = new MockFilterChain();
151162
requestMock.setSession(loginAsAnonymous());
163+
152164
return toTest;
153165
}
154166
}

‎domain/src/main/java/org/fao/geonet/domain/AnonymousAccessLink.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,11 @@ public int hashCode() {
102102
return Objects.hash(id, metadataId, metadataUuid, hash);
103103
}
104104

105+
public static int getRandomHashLength() {
106+
return 43;
107+
}
105108
public static String getRandomHash() {
106-
BytesKeyGenerator generator = KeyGenerators.secureRandom(64);
107-
return Base64.getUrlEncoder().encodeToString(generator.generateKey());
109+
BytesKeyGenerator generator = KeyGenerators.secureRandom(32);
110+
return Base64.getUrlEncoder().withoutPadding().encodeToString(generator.generateKey());
108111
}
109112
}

‎domain/src/main/java/org/fao/geonet/repository/AnonymousAccessLinkRepository.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@
2727

2828
public interface AnonymousAccessLinkRepository extends GeonetRepository<AnonymousAccessLink, Integer> {
2929

30-
AnonymousAccessLink findOneByHash(String hash);
31-
3230
AnonymousAccessLink findOneByMetadataUuid(String uuid);
3331

3432
}

‎domain/src/test/java/org/fao/geonet/repository/AnonymousAccessLinkRepositoryTest.java‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,8 @@
2929
import org.springframework.beans.factory.annotation.Autowired;
3030

3131
import java.util.List;
32-
import java.util.Optional;
3332

3433
import static org.junit.Assert.assertEquals;
35-
import static org.junit.Assert.assertNotNull;
36-
import static org.junit.Assert.assertNull;
3734
import static org.junit.Assert.assertTrue;
3835

3936
public class AnonymousAccessLinkRepositoryTest extends AbstractSpringDataTest {
@@ -66,22 +63,4 @@ public void nominal() {
6663
assertEquals(0, accesses.size());
6764
}
6865

69-
@Test
70-
public void findByHash() {
71-
AnonymousAccessLink anonymousAccessLink = new AnonymousAccessLink() //
72-
.setMetadataId(666) //
73-
.setMetadataUuid("uuid") //
74-
.setHash("hash");
75-
anonymousAccessLinkRepository.save(anonymousAccessLink);
76-
77-
AnonymousAccessLink auth = anonymousAccessLinkRepository.findOneByHash("hash");
78-
79-
assertNotNull(auth);
80-
assertEquals(666, auth.getMetadataId());
81-
82-
auth = anonymousAccessLinkRepository.findOneByHash("hush");
83-
84-
assertNull(auth);
85-
}
86-
8766
}

‎services/src/main/java/org/fao/geonet/api/anonymousAccessLink/AnonymousAccessLinkService.java‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,11 @@
3333
import org.fao.geonet.kernel.datamanager.IMetadataUtils;
3434
import org.fao.geonet.repository.AnonymousAccessLinkRepository;
3535
import org.fao.geonet.repository.OperationAllowedRepository;
36+
import org.fao.geonet.util.PasswordUtil;
3637
import org.springframework.beans.factory.annotation.Autowired;
38+
import org.springframework.beans.factory.annotation.Qualifier;
3739
import org.springframework.beans.factory.annotation.Value;
40+
import org.springframework.security.crypto.password.PasswordEncoder;
3841
import org.springframework.stereotype.Service;
3942

4043
import java.io.IOException;
@@ -63,6 +66,10 @@ public class AnonymousAccessLinkService {
6366
@Autowired
6467
private OperationAllowedRepository opAllowedRepo;
6568

69+
@Autowired
70+
@Qualifier(PasswordUtil.ENCODER_ID)
71+
private PasswordEncoder encoder;
72+
6673
public AnonymousAccessLinkDto createAnonymousAccessLink(String uuid) throws ResourceAlreadyExistException {
6774
if (anonymousAccessLinkRepository.findOneByMetadataUuid(uuid) != null) {
6875
throw new ResourceAlreadyExistException();
@@ -75,15 +82,19 @@ public AnonymousAccessLinkDto createAnonymousAccessLink(String uuid) throws Reso
7582
AnonymousAccessLink anonymousAccessLinkToCreate = new AnonymousAccessLink()
7683
.setMetadataId(mdId)
7784
.setMetadataUuid(uuid)
78-
.setHash(randomHash);
85+
.setHash(encoder.encode(randomHash));
7986
anonymousAccessLinkRepository.save(anonymousAccessLinkToCreate);
80-
return mapper.toDto(anonymousAccessLinkToCreate).setHash(randomHash);
87+
return mapper.toDto(anonymousAccessLinkToCreate).setHash(String.format("%s%s", randomHash, uuid));
8188
}
8289

8390
public List<AnonymousAccessLinkDto> getAllAnonymousAccessLinksWithMdInfos() throws IOException {
8491
List<AnonymousAccessLink> anonymousAccessLinks = anonymousAccessLinkRepository.findAll();
8592

8693
List<String> uuids = anonymousAccessLinks.stream().map(AnonymousAccessLink::getMetadataUuid).collect(Collectors.toList());
94+
if (uuids.isEmpty()) {
95+
return List.of();
96+
}
97+
8798
MgetRequest request = new MgetRequest.Builder()
8899
.ids(uuids)
89100
.index(defaultIndex)

‎services/src/test/java/org/fao/geonet/api/anonymousAccessLink/AnonymousAccessLinkServiceTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public void createAnonymousAccessLink() throws Exception {
6969
AnonymousAccessLink stored = anonymousAccessLinkRepository.findOneByMetadataUuid(md.getUuid());
7070
assertEquals(stored.getMetadataUuid(), created.getMetadataUuid());
7171
assertEquals(stored.getMetadataId(), created.getMetadataId());
72-
assertEquals(stored.getHash(), created.getHash());
72+
assertEquals(stored.getHash() + stored.getMetadataUuid(), created.getHash());
7373
}
7474

7575
@Test

0 commit comments

Comments
 (0)