You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nifi.apache.org by mcgilman <gi...@git.apache.org> on 2016/04/06 15:46:19 UTC

[GitHub] nifi pull request: NIFI-1553: File based authorizer

GitHub user mcgilman opened a pull request:

    https://github.com/apache/nifi/pull/330

    NIFI-1553: File based authorizer

    - Implementing a file based authorizer.
    - Providing an example authorizations files.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/mcgilman/nifi NIFI-1553

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/nifi/pull/330.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #330
    
----
commit e994cc6f1b596be65498d5c7cbd9b5f9f4689026
Author: Matt Gilman <ma...@gmail.com>
Date:   2016-04-05T20:43:56Z

    NIFI-1553:
    - Implementing a file based authorizer.
    - Providing an example authorizations files.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1553: File based authorizer

Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the pull request:

    https://github.com/apache/nifi/pull/330#issuecomment-207070409
  
    Awesome thanks. FYI - I'm removing one line of dead code from the FileAuthorizerTest prior to merging to master.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1553: File based authorizer

Posted by markap14 <gi...@git.apache.org>.
Github user markap14 commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/330#discussion_r58933064
  
    --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/test/java/org/apache/nifi/authorization/FileAuthorizerTest.java ---
    @@ -0,0 +1,207 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.nifi.authorization;
    +
    +import org.apache.nifi.attribute.expression.language.StandardPropertyValue;
    +import org.apache.nifi.authorization.AuthorizationResult.Result;
    +import org.apache.nifi.authorization.exception.ProviderCreationException;
    +import org.apache.nifi.authorization.resource.ResourceFactory;
    +import org.apache.nifi.util.NiFiProperties;
    +import org.apache.nifi.util.file.FileUtils;
    +import org.junit.After;
    +import org.junit.Before;
    +import org.junit.Test;
    +import org.mockito.Mockito;
    +
    +import java.io.File;
    +import java.io.FileOutputStream;
    +import java.io.IOException;
    +import java.nio.charset.StandardCharsets;
    +
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.assertFalse;
    +import static org.junit.Assert.assertTrue;
    +import static org.mockito.Mockito.mock;
    +import static org.mockito.Mockito.when;
    +
    +public class FileAuthorizerTest {
    +
    +    private static final String EMPTY_AUTHORIZATIONS_CONCISE =
    +        "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>"
    +        + "<resources/>";
    +
    +    private static final String EMPTY_AUTHORIZATIONS =
    +        "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>"
    +        + "<resources>"
    +        + "</resources>";
    +
    +    private static final String BAD_SCHEMA_AUTHORIZATIONS =
    +        "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>"
    +        + "<resource>"
    +        + "</resource>";
    +
    +    private static final String AUTHORIZATIONS =
    +        "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>"
    +        + "<resources>"
    +            + "<resource identifier=\"/flow\">"
    +                + "<authorization identity=\"user-1\" action=\"R\"/>"
    +            + "</resource>"
    +        + "</resources>";
    +
    +    private static final String UPDATED_AUTHORIZATIONS =
    +        "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>"
    +        + "<resources>"
    +            + "<resource identifier=\"/flow\">"
    +                + "<authorization identity=\"user-1\" action=\"RW\"/>"
    +            + "</resource>"
    +        + "</resources>";
    +
    +    private FileAuthorizer authorizer;
    +    private File primary;
    +    private File restore;
    +
    +    private AuthorizerConfigurationContext configurationContext;
    +
    +    @Before
    +    public void setup() throws IOException {
    +        // primary authorizations
    +        primary = new File("target/primary/authorizations.xml");
    +        FileUtils.ensureDirectoryExistAndCanAccess(primary.getParentFile());
    +
    +        // restore authorizations
    +        restore = new File("target/restore/authorizations.xml");
    +        FileUtils.ensureDirectoryExistAndCanAccess(restore.getParentFile());
    +
    +        final NiFiProperties properties = mock(NiFiProperties.class);
    +        when(properties.getRestoreDirectory()).thenReturn(restore.getParentFile());
    +
    +        configurationContext = mock(AuthorizerConfigurationContext.class);
    +        when(configurationContext.getProperty(Mockito.eq("Authorizations File"))).thenReturn(new StandardPropertyValue(primary.getPath(), null));
    +
    +        authorizer = new FileAuthorizer();
    +        authorizer.setNiFiProperties(properties);
    +        authorizer.initialize(null);
    +    }
    +
    +    @After
    +    public void cleanup() throws Exception {
    +        deleteFile(primary);
    +        deleteFile(restore);
    +    }
    +
    +    @Test
    +    public void testPostContructionWhenRestoreDoesNotExist() throws Exception {
    +        writeAuthorizationsFile(primary, EMPTY_AUTHORIZATIONS_CONCISE);
    +        authorizer.onConfigured(configurationContext);
    +
    +        assertEquals(primary.length(), restore.length());
    +    }
    +
    +    @Test
    +    public void testPostConstructionWhenPrimaryDoesNotExist() throws Exception {
    +        writeAuthorizationsFile(restore, EMPTY_AUTHORIZATIONS_CONCISE);
    +        authorizer.onConfigured(configurationContext);
    +
    +        assertEquals(restore.length(), primary.length());
    +    }
    +
    +    @Test(expected = ProviderCreationException.class)
    +    public void testPostConstructionWhenPrimaryDifferentThanRestore() throws Exception {
    +        writeAuthorizationsFile(primary, EMPTY_AUTHORIZATIONS);
    +        writeAuthorizationsFile(restore, EMPTY_AUTHORIZATIONS_CONCISE);
    +        authorizer.onConfigured(configurationContext);
    +    }
    +
    +    @Test
    +    public void testPostConstructionWhenPrimaryAndBackupDoNotExist() throws Exception {
    +        authorizer.onConfigured(configurationContext);
    +        assertEquals(0, restore.length());
    +        assertEquals(restore.length(), primary.length());
    +    }
    +
    +    @Test(expected = ProviderCreationException.class)
    +    public void testBadSchema() throws Exception {
    +        writeAuthorizationsFile(primary, BAD_SCHEMA_AUTHORIZATIONS);
    +        authorizer.onConfigured(configurationContext);
    +    }
    +
    +    @Test
    +    public void testAuthorizedUserAction() throws Exception {
    +        writeAuthorizationsFile(primary, AUTHORIZATIONS);
    +        authorizer.onConfigured(configurationContext);
    +
    +        final AuthorizationRequest request = new AuthorizationRequest.Builder().resource(ResourceFactory.getFlowResource()).identity("user-1").action(RequestAction.READ).build();
    +        final AuthorizationResult result = authorizer.authorize(request);
    +        assertTrue(Result.Approved.equals(result.getResult()));
    +    }
    +
    +    @Test
    +    public void testUnauthorizedUser() throws Exception {
    +        writeAuthorizationsFile(primary, AUTHORIZATIONS);
    +        authorizer.onConfigured(configurationContext);
    +
    +        final AuthorizationRequest request = new AuthorizationRequest.Builder().resource(ResourceFactory.getFlowResource()).identity("user-2").action(RequestAction.READ).build();
    +        final AuthorizationResult result = authorizer.authorize(request);
    +        assertFalse(Result.Approved.equals(result.getResult()));
    +    }
    +
    +    @Test
    +    public void testUnauthorizedAction() throws Exception {
    +        writeAuthorizationsFile(primary, AUTHORIZATIONS);
    +        authorizer.onConfigured(configurationContext);
    +
    +        final AuthorizationRequest request = new AuthorizationRequest.Builder().resource(ResourceFactory.getFlowResource()).identity("user-1").action(RequestAction.WRITE).build();
    +        final AuthorizationResult result = authorizer.authorize(request);
    +        assertFalse(Result.Approved.equals(result.getResult()));
    +    }
    +
    +    @Test
    +    public void testReloadAuthorizations() throws Exception {
    +        writeAuthorizationsFile(primary, AUTHORIZATIONS);
    +        when(configurationContext.getProperty(Mockito.eq("Reload Interval"))).thenReturn(new StandardPropertyValue("1 sec", null));
    +        authorizer.onConfigured(configurationContext);
    +
    +        // ensure the user currently does not have write access
    +        final AuthorizationRequest request = new AuthorizationRequest.Builder().resource(ResourceFactory.getFlowResource()).identity("user-1").action(RequestAction.WRITE).build();
    +        AuthorizationResult result = authorizer.authorize(request);
    +        assertFalse(Result.Approved.equals(result.getResult()));
    +
    +        // add write access for the user
    +        writeAuthorizationsFile(primary, UPDATED_AUTHORIZATIONS);
    +
    +        // wait at least one second for the file to be stale
    +        Thread.sleep(4000L);
    +
    +        // ensure the user does have write access now using the same request
    +        result = authorizer.authorize(request);
    +        assertTrue(Result.Approved.equals(result.getResult()));
    +    }
    +
    +    private static void writeAuthorizationsFile(final File file, final String content) throws Exception {
    +        byte[] bytes = content.getBytes(StandardCharsets.UTF_8);
    +        FileOutputStream fos = new FileOutputStream(file);
    --- End diff --
    
    Should probably use a try-with-resources here:
    try (final FileOutputStream fos = new FileOutputStream(file)) {
      fos.write(bytes);
    }
    
    Or, alternatively, just simply: Files.write(bytes, file);


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1553: File based authorizer

Posted by markap14 <gi...@git.apache.org>.
Github user markap14 commented on the pull request:

    https://github.com/apache/nifi/pull/330#issuecomment-207066913
  
    Latest commit looks good. Performed code review and then verified contrib-check and unit tests. +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1553: File based authorizer

Posted by markap14 <gi...@git.apache.org>.
Github user markap14 commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/330#discussion_r58929464
  
    --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-administration/src/main/java/org/apache/nifi/authorization/StandardAuthorizerConfigurationContext.java ---
    @@ -44,8 +47,13 @@ public String getIdentifier() {
         }
     
         @Override
    -    public String getProperty(String property) {
    -        return properties.get(property);
    +    public PropertyValue getProperty(String property) {
    +        final String value = properties.get(property);
    +        if (value == null) {
    --- End diff --
    
    In this case, I think we should still be returning a new StandardPropertyValue with a value of null, rather than returning no PropertyValue at all. This allows us to chain method calls, such as: Integer x = getProperty(...).asInteger();


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1553: File based authorizer

Posted by markap14 <gi...@git.apache.org>.
Github user markap14 commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/330#discussion_r58930317
  
    --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/FileAuthorizer.java ---
    @@ -0,0 +1,278 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.nifi.authorization;
    +
    +import org.apache.nifi.authorization.annotation.AuthorizerContext;
    +import org.apache.nifi.authorization.exception.AuthorizationAccessException;
    +import org.apache.nifi.authorization.exception.ProviderCreationException;
    +import org.apache.nifi.authorization.generated.Authorization;
    +import org.apache.nifi.authorization.generated.Resource;
    +import org.apache.nifi.authorization.generated.Resources;
    +import org.apache.nifi.components.PropertyValue;
    +import org.apache.nifi.util.NiFiProperties;
    +import org.apache.nifi.util.StringUtils;
    +import org.apache.nifi.util.file.FileUtils;
    +import org.apache.nifi.util.file.monitor.MD5SumMonitor;
    +import org.apache.nifi.util.file.monitor.SynchronousFileWatcher;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +import org.xml.sax.SAXException;
    +
    +import javax.xml.XMLConstants;
    +import javax.xml.bind.JAXBContext;
    +import javax.xml.bind.JAXBElement;
    +import javax.xml.bind.JAXBException;
    +import javax.xml.bind.Unmarshaller;
    +import javax.xml.transform.stream.StreamSource;
    +import javax.xml.validation.Schema;
    +import javax.xml.validation.SchemaFactory;
    +import java.io.File;
    +import java.io.IOException;
    +import java.util.Date;
    +import java.util.EnumSet;
    +import java.util.HashMap;
    +import java.util.Map;
    +import java.util.Set;
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.ScheduledExecutorService;
    +import java.util.concurrent.ThreadFactory;
    +import java.util.concurrent.TimeUnit;
    +import java.util.concurrent.atomic.AtomicReference;
    +
    +/**
    + * Provides identity checks and grants authorities.
    + */
    +public class FileAuthorizer implements Authorizer {
    +
    +    private static final Logger logger = LoggerFactory.getLogger(FileAuthorizer.class);
    +    private static final String READ_CODE = "R";
    +    private static final String WRITE_CODE = "W";
    +    private static final String USERS_XSD = "/authorizations.xsd";
    +    private static final String JAXB_GENERATED_PATH = "org.apache.nifi.authorization.generated";
    +    private static final JAXBContext JAXB_CONTEXT = initializeJaxbContext();
    +
    +    /**
    +     * Load the JAXBContext.
    +     */
    +    private static JAXBContext initializeJaxbContext() {
    +        try {
    +            return JAXBContext.newInstance(JAXB_GENERATED_PATH, FileAuthorizer.class.getClassLoader());
    +        } catch (JAXBException e) {
    +            throw new RuntimeException("Unable to create JAXBContext.");
    +        }
    +    }
    +
    +    private NiFiProperties properties;
    +    private File authorizationsFile;
    +    private File restoreAuthorizationsFile;
    +    private SynchronousFileWatcher fileWatcher;
    +    private ScheduledExecutorService fileWatcherExecutorService;
    +
    +    private final AtomicReference<Map<String, Map<String, Set<RequestAction>>>> authorizations = new AtomicReference<>();
    +
    +    @Override
    +    public void initialize(final AuthorizerInitializationContext initializationContext) throws ProviderCreationException {
    +    }
    +
    +    @Override
    +    public void onConfigured(final AuthorizerConfigurationContext configurationContext) throws ProviderCreationException {
    +        try {
    +            final PropertyValue authorizationsPath = configurationContext.getProperty("Authorizations File");
    +            if (authorizationsPath == null || StringUtils.isBlank(authorizationsPath.getValue())) {
    +                throw new ProviderCreationException("The authorized users file must be specified.");
    +            }
    +
    +            authorizationsFile = new File(authorizationsPath.getValue());
    +            final File authorizationsFileDirectory = authorizationsFile.getParentFile();
    --- End diff --
    
    If authorizationsFile is set to something like "myFile.txt" without a directory, calling getParentFile() will return null. This would then cause a NullPointerException below. We should probably instead use:
    {code}
    final File authorizationsFileDirectory = authorizationsFile.getAbsoluteFile().getParentFile();
    {code}


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1553: File based authorizer

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/nifi/pull/330


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---