Cool, I'd love to hear other thoughts as well. Just to add more context, I'd like to see the ReflectionBuilder logic separated from the IniSecurityManagerFactory. Currently the ReflectionBuilder and the SecurityManager creation are tightly coupled. Starting in 1.4, we added a `getReflectionBuilder()` method to allow more customization of the ReflectionBuilder and it's configuration. This is not ideal, but allows us to keep backwards compatibility.
Then us adding something like a `defaults-shiro.ini` that would define all/most of the default configuration (for example SecurityManager implementation). Then we wouldn't need have a Web and non Web implementation. (and the Web Ini Environment could just layer in another ini file to add anything servlet specific. Sorry for the ramble, just trying to add some thoughts to the discussion. -Brian On Wed, Jun 7, 2017 at 8:49 AM, Dan Dold <[email protected]> wrote: > Ah I understand, your taking it away from the public view by making it an > internal dependency. > Is this the only solution for this problem or are there others? > > I can do what you suggested Brian, but I would like to see if others has > any other input first? > > On Mon, Jun 5, 2017 at 10:31 PM, Brian Demers <[email protected]> > wrote: > > > I agree it is a short term fix, but it makes the usage of the > > IniSecurityManagerFactory > > an internal dependency. > > > > From here we can move/refactor as we see fit. But the class is still > > deprecated from the point of the public API. > > Granted this still leaves a some code debt, but we cannot remove that > class > > until 2.0 anyway. > > > > I'm all for other options too :) > > > > On Mon, Jun 5, 2017 at 1:29 PM, Dan Dold <[email protected]> wrote: > > > > > Hi Brian, > > > > > > BasicIniEnvironment uses IniSecurityManagerFactory in its constructor. > > > Is this not just a roundabout way of using IniSecurityManagerFactory > > which > > > defeats the purpose as, I assume, in a few versions the > > > IniSecurityManagerFactory > > > class will be removed and then this issue will be faced with again. > > > > > > The solution you proposed looks like a quick fix rather than a long > term > > > solution, would it not be better to look into Environment impl without > > the > > > use of IniSecurityManagerFactory? > > > > > > On Sat, Jun 3, 2017 at 3:15 AM, Brian Demers <[email protected]> > > > wrote: > > > > > > > Hey Dan, > > > > > > > > Thanks for looking into this! It looks like we don't have a good > > > > replacement for a non-Servlet/Spring/Guice Environment impl. We > > probably > > > > need something like a BasicIniEnvironment > > > > > > > > https://gist.github.com/bdemers/7acede56023b7f879a1103bc5373a2c5 > > > > > > > > /** > > > > * Basic usage:<p> > > > > * <code> > > > > * Environment env = new BasicIniEnvironment("classpath:shiro.ini"); > > > > * SecurityManager securityManager = env.getSecurityManager(); > > > > * </code> > > > > * > > > > */ > > > > public class BasicIniEnvironment extends DefaultEnvironment { > > > > > > > > public BasicIniEnvironment(Ini ini) { > > > > setSecurityManager(new IniSecurityManagerFactory(ini) > > > > .getInstance()); > > > > } > > > > > > > > public BasicIniEnvironment(String iniResourcePath) { > > > > this(Ini.fromResourcePath(iniResourcePath)); > > > > } > > > > } > > > > > > > > We are still using the a derivative of an IniSecurityManger in the > Web > > > impl > > > > as well. So something like this would at least hide the > implementation > > in > > > > the Environment. > > > > > > > > Thoughts? > > > > > > > > > > > > On Thu, Jun 1, 2017 at 5:30 PM, Dan Dold <[email protected]> wrote: > > > > > > > > > Hi, > > > > > > > > > > I am trying to work on SHIRO-625 ticket and I am finding it hard to > > > > > understand the new implementation. > > > > > > > > > > I understand the deprecated code; > > > > > > > > > > > Factory<SecurityManager> factory = new > IniSecurityManagerFactory(" > > > > > > classpath:shiro.ini") > > > > > > > > > > SecurityManager securityManager = factory.getInstance() > > > > > > > > > > > > > > > This is a simple implementation but looking up how to implement the > > > > > Environment class in a similar fashion is proving very difficult. > > > > > So far I have got as far as; > > > > > > > > > > > Ini config = Ini.fromResourcePath("classpath;shiro:ini") > > > > > > > > > > Environment environment = new DefaultEnvironment(config) > > > > > > > > > > SecurityManager securityManager = environment.getSecurityManager() > > > > > > > > > > > > > > > But running this produces the following error; > > > > > > > > > > > Exception in thread "main" java.lang.IllegalStateException: No > > > > > >> SecurityManager found in Environment. This is an invalid > > > environment > > > > > state. > > > > > > > > > > > > at > > > > > >> org.apache.shiro.env.DefaultEnvironment.getSecurityManager( > > > > > DefaultEnvironment.java:81) > > > > > > > > > > > > at Quickstart.main(Quickstart.java:60) > > > > > > > > > > > > > > > > > I have tried setting the SecurityManager doing the following; > > > > > > > > > > > environment.setSecurityManager(new DefaultSecurityManager()); > > > > > > > > > > > > > > > But that gave the following error; > > > > > > > > > > > > > > > > > > > > > Exception in thread "main" java.lang.ClassCastException: > > > > > > org.apache.shiro.mgt.DefaultSecurityManager cannot be cast to > > > > > > org.apache.shiro.config.Ini$Section > > > > > > > > > > at org.apache.shiro.config.Ini.put(Ini.java:47) > > > > > > > > > > at > > > > > > org.apache.shiro.env.DefaultEnvironment.setObject( > > > > > DefaultEnvironment.java:162) > > > > > > > > > > at > > > > > > org.apache.shiro.env.DefaultEnvironment.setSecurityManager( > > > > > DefaultEnvironment.java:92) > > > > > > > > > > at Quickstart.main(Quickstart.java:60) > > > > > > > > > > > > > > > I'm completely stuck on where to turn now. > > > > > Is there any documentation on how to setup Shiro using the > Enviroment > > > (or > > > > > is that what I am trying to setup), even a quick explanation on > > where I > > > > am > > > > > going right/wrong. > > > > > > > > > > Thanks, > > > > > Dan > > > > > > > > > > > > > > >
